perf(semantic): use FxHashMap instead of IndexMap#4228
Conversation
Your org has enabled the Graphite merge queue for merging into mainAdd the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
CodSpeed Performance ReportMerging #4228 will improve performances by 3.37%Comparing Summary
Benchmarks breakdown
|
|
I did wonder why we were using |
Dunqing
force-pushed
the
07-12-perf_semantic_use_fxhashmap_instead_of_indexmap
branch
from
4f2efd6 to
3234279
Compare
I don't know why. I just found it slower than |
|
We got 1%~3% performance improvement and removed 1 dependency! |
Dunqing
force-pushed
the
07-12-perf_semantic_use_fxhashmap_instead_of_indexmap
branch
from
3234279 to
f1e78e8
Compare
Dunqing
changed the title
FxHashMap instead of indexmapFxHashMap instead of IndexMap
|
The mangler is sensitive to binding position. |
Ah ha. So that's why But... binding insertion order and position don't always align. They will if the AST comes direct from the parser. But what if a transform alters e.g. function params? Then you could end up with the 1st param being the last binding. |
|
oxc/crates/oxc_mangler/src/lib.rs Line 71 in be82c28 Symbols and scopes are recreated, since mangler is the last stage just before codegen of the pipeline, even after the compressor. |
|
I assume (perhaps incorrectly?) that our intention is that ultimately transform and compress will update symbols and scopes as they modify the AST, so we won't need to keep recreating them. So at that point, the binding insertion order / position mismatch could become a problem. But perhaps we should leave this as is for now. |
|
Close this because we're not sure yet that we can safely replace it with a |
Boshen
deleted the
07-12-perf_semantic_use_fxhashmap_instead_of_indexmap
branch
`IndexMap` was needed for the insertion order requirement in mangler. Bindings (symbol ids) are monotonically increasing by binding positions, which means get insertion order by sorting the symbol ids in mangler. Previous attempt: #4228
`IndexMap` was needed for the insertion order requirement in mangler. Bindings (symbol ids) are monotonically increasing by binding positions, which means we can get insertion order by sorting the symbol ids in mangler. Previous attempt: #4228
3 participants
No description provided.