perf(semantic): reduce index_vec allocation

[Dunqing]

Combining some SymbolTable fields to the Symbol struct, we can reduce allocating IndexVec. After my changes, I suddenly realized the api is much easier to use now. We no longer need the symbol_id to get the various field value you want, you just need the Symbol

[graphite-app]

Your org has enabled the Graphite merge queue for merging into main

Add 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.

[Dunqing]

• perf(semantic): reduce index_vec allocation #4266 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @Dunqing and the rest of your teammates on Graphite

[codspeed-hq]

CodSpeed Performance Report

Merging #4266 will not alter performance

_{Comparing 07-15-perf_semantic_reduce_index_vec_allocation (29f251a) with main (639fd48)}

Summary

✅ 32 untouched benchmarks

[overlookmotel]

Interesting that this change from struct-of-arrays to array-of-structs is a gain on benchmarks. However, there is another possible approach to tackling this which may be even better: oxc-project/backlog#11

However, since this PR is a perf gain at present, we could merge it now and attempt oxc-project/backlog#11 later on?

#3318 (comment) is also relevant, though that's pretty tricky - we'd probably need "branded lifetimes" to make it safe.

[Dunqing]

Interesting that this change from struct-of-arrays to array-of-structs is a gain on benchmarks. However, there is another possible approach to tackling this which may be even better: oxc-project/backlog#11

However, since this PR is a gain at present, we could merge it now and attempt oxc-project/backlog#11 later on?

This PR is just an attempt at my guess, I'm not sure we want to merge it. I know you will be optimizing the whole semantic, and you probably have further improvements that can be done better. So it's up to you whether to merge it or not!

[Boshen]

SoA improves write performance but regresses read performance (notice drop in linter).

I think we should keep SoA unless there is a win across the table + Symbol byte size won't increase in the future.

[Dunqing]

SoA improves write performance but regresses read performance (notice drop in linter).

There are further optimizations we can make in linter, Such as we can pass Symbol to run_on_symbol. This may give a small performance boost

I think we should keep SoA unless there is a win across the table + Symbol byte size won't increase in the future.

I realize this, it's something we need to think about

[DonIsaac]

Should we encapsulate these fields for future flexibility?

[DonIsaac]

Consider adding a GetSpan impl.

[Dunqing]

Closing this because we tried another better way to improve more in #4328