-
-
Notifications
You must be signed in to change notification settings - Fork 793
perf: store types behind Arcs
#6442
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
Conversation
|
CodSpeed Performance ReportMerging #6442 will create unknown performance changesComparing Summary
Benchmarks breakdown
|
Summary
This PR implements 3 different performance optimisations, one of which has a large impact on the code, and one of which has a large impact on the performance.
TypeStorenow stores all itsTypeDatainstances behindArcs. To facilitate this, the signatures forTypeData::resolved()andTypeData::flattened()have been updated and both return anOption<TypeData>now, and only whenSomeinstance is returned we create a newArcfor it. This approach saves a significant amount of cloning during the flattening and resolving phases. Moreover, the code became significantly simpler and less scary because types can now be resolved and flattened while they're still part of the store, so there's no need for "take type temporarily" workarounds anymore. All in all, this change made Biome's total running time ~3% faster.ModuleResolverwe have amodules_by_pathmap to quickly find imported modules. It seems the amount of modules we import has become quite considerable, because I was able to achieve a ~30% performance improvement by changing this from aBTreeMapto anFxHashMap.I have dropped the React snapshot from the tests, since it was creating a lot of noise and the purpose of the snapshot (improving inference from React imports) has been achieved.
Test Plan
Everything should stay green.