feat(semantic): track cfg index per ast node

[TzviPM]

This allows looking up a cfg index from an ast node in a semantics return. This allows later passes to better make use of the cfg.

[codspeed-hq]

CodSpeed Performance Report

Merging #2210 will degrade performances by 3.02%

_{Comparing TzviPM:01-29-feat_semantic_track_cfg_index_per_ast_node (25385be) with main (622a2c3)}

Summary

❌ 1 (👁 1) regressions
✅ 16 untouched benchmarks

Benchmarks breakdown

	Benchmark	main	TzviPM:01-29-feat_semantic_track_cfg_index_per_ast_node	Change
👁	semantic[antd.js]	750.7 ms	774 ms	-3.02%

[u9g]

You’re doing a ton more work to store these in a hashmap still, maybe its worth making this an indexmap using the same AstNodeId as keytype? Still you are doing a lot more work and there will be a perf regression here. I’d be curious to know what specific new usecase this enables.

[TzviPM]

Now that I have write access, I will try to use graphite inside of oxc directly to manage PR chains. I was doing this part in my own fork, but you can see my WIP of using this code in TzviPM#5. That PR is currently failing because of CFG issues (I think) and I will tune the performance of this PR once I have the use case working as well.

[TzviPM]

You’re doing a ton more work to store these in a hashmap still, maybe its worth making this an indexmap using the same AstNodeId as keytype? Still you are doing a lot more work and there will be a perf regression here. I’d be curious to know what specific new usecase this enables.

I think the problem is 2-fold:

1. As you mentioned, an index map makes sense here.

2. I introduced an additional branch inside of a hotpath. I wonder if it would be better to always store the current index, and then just correct it after for functions. In this way, the cpu won't suffer from branch prediction failures. Initially, I thought the cpu would be smart enough to just predict the next branch better based on this one (in enter_kind), but thtlat may have been too much faith in an ol' bucket of bolts 😊

[TzviPM]

At the end of the day, I'm not calculating anything extra, just storing an association that's already calculated. Im sure there's a way to do this without a 40% performance degredation.

[TzviPM]

Ooh, what if the ast node was actually not a u32, but actually a u24 with a u8. Basically, we always create an ast node before a node index, and then we associate the ast node with the created index. If we create the index first, then every time we make a node, we store it's ID as the node index (currently 32 bits, but just 24 bits might be enough) followed by an ast node inner index which denotes the position of the ast node within the CFG node.

In this way, we don't need to store any additional data on the heap, just make some in-register calculations to go from an ast node ID to the CFG node index.

[Boshen]

Changed to draft due to performance regression.

Please excuse me for not understanding any of this right now. The only blocker from myside is the performance regression and nothing else.

[TzviPM]

TzviPM#6 is the downstream PR that implements no_this_before_super. Now that I have everything wired up trivially, I will start tweaking the solution to fix the performance regression.

[TzviPM]

Okay, so indexmap definitely helped. I tried removing the conditional, and it actually did regress further:

[TzviPM]

My initial removing of the conditional ended up always double-setting the function nodes. I fixed that by simply re-ordering. Then, instead of having a separate node map, I tried storing the cfg index right on the astnode of the semantic struct. This also makes more sense to me in terms of separation of concerns and boosted performance dramatically.