Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using private methods has some performance impact #72

Open
alxvasilev opened this issue Oct 6, 2024 · 2 comments
Open

Using private methods has some performance impact #72

alxvasilev opened this issue Oct 6, 2024 · 2 comments

Comments

@alxvasilev
Copy link

alxvasilev commented Oct 6, 2024

Looking at the provided bundles, I discovered that some of the frequently executed code is full of calls to private property access check helpers, which are generated by esbuild. I tried to have these removed, and the properties/methods treated as usual, assuming names starting with # are backward compatible. I tried with esbuild and with webpack + typescript, but couldn't achieve that without bumping the target ES version to >= 2022 (I have zero experience with esbuild, though). Having the target as ES2022 or ESNEXT solved this problem, but leaves other "too-modern" features as well, such as optional chanining, which is unacceptable in my case. I noticed that the webpack-built bundle with the checks has a noticeable increase in CPU usage, while your pre-built bundle seems to be just a tad slower, almost within the measurement error. The test I did was encoding a 720p video stream of 16-30fps and audio chunks sent to the muxer every 20ms, which makes about 60-70 calls to addAudio/VideoChunk per second.
Overall, you may want to have a look at these accessor calls, as they really look like unnecessary overhead and I'm not sure if they are worth any performance penalty.

@alxvasilev alxvasilev changed the title Using private methods has non-negligible performance impact Using private methods has some performance impact Oct 6, 2024
@Vanilagy
Copy link
Owner

Vanilagy commented Oct 7, 2024

Oh wait, really? I mean I always thought these private property accessors read a bit weird, but at least they minify well. It's definitely more expensive than the untransformed version, but is the impact really so great? Do you have any concrete measurements? I would assume that the actual time spent in the muxer code is very little compared to, yknow, the amount of work it takes to encode a video. Of course if there's an actual performance hit, that's something I need to address because I care about performance.

@alxvasilev
Copy link
Author

alxvasilev commented Oct 17, 2024

It's very tricky to do exact measurements, but here is my attempt. What I did is measured the average and minimum CPU load of the Chrome process that runs my web application (in Chrome's task manager), while it is recording audio only. The reason it's audio-only is to eliminate the significant measurement noise of CPU-heavy video encoding, while leaving the majority of muxer calls, as audio packets are generated at approx twice the rate of video packets (50 per second).
Just for comparison, I created a third case, where I also removed all validations of input parameters in addAudioChunk() and addAudioChunkRaw():

The results are:

original npm package:    avg 73%, min 68%
no privacy checks:       avg 65%, min 60%
no priv & no validate:   avg 63%  min 58%

As you can see, removing the privacy checks resulted in ~8% less CPU load. The various input param validations have a much smaller overhead of ~2%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants