-
Notifications
You must be signed in to change notification settings - Fork 179
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
Speed up LoadPathCache with more C #278
Conversation
57006ef
to
fb28b2f
Compare
Rebased master to resolve conflict |
Is there any ETA when this gets merged? We would profit from this probably ❤️ |
@h0jeZvgoxFepBQ2C this now conflicts with changes from #312. I think even with the other PR merged it would be still beneficial to have an option to exclude some dirs (node_modules, cache). I can extract it from this one and create another pull request if the maintainers are interested. Pure C dir walker of course still would be faster than current master version, but the combination of changes in #312 with exclusion option might be enough to give significant speedup without the necessity to maintain scary chunk of C code. |
Is this closed since its completed? or whats the reasoning? |
The important part, which is skipping big useless directories like As for directory scanning in C, I doesn't seem like it yields enough of a speedup to justify the increased complexity. Additionally the branch was very udated so the rebasing would have been tricky. |
I decided to keep up the momentum with #277 and create a tenative pull request with my proposed changes to load path cache.
This includes:
Dir.glob
. It is both faster and easier on memory - unlikeDir.glob
creating a large array and then iterating it, it yields with every found path and does path exclusion on C level.I tested those changes against Discourse, but without significant speedup:
However, with my internal project with a lot of javascript (webpacker etc.), caching etc. savings are significant:
Legend:
rm tmp/cache/bootsnap-load-path-cache
)I'm not the best C programmer (or even mediocre one), so to use this new C extension one must set environment variable
BOOTSNAP_EXPERIMENTAL
. This way people with project in dire need to speed up can experimentally use this, while most of the people can safely use existing version.Let me know what you think about this direction.