search

glimmerjs / glimmer-vm

MIT License
1.13k stars 191 forks source link

Move from const enums to consts #1645

Closed wycats closed 2 weeks ago

wycats commented 3 weeks ago

This PR ended up being much more extensive than originally intended because it ended up including quite a bit of cleanup to the benchmark setup and cleaned up mistakes in the package.jsons that were causing subtle issues in the benchmark environment.

[EDIT] @NullVoxPopuli pushed me to extract a lot of the incidental changes into their own PRs. I did quite a bit of that and it did streamline this PR. Thanks @NullVoxPopuli!

wycats commented 2 weeks ago

image

Local results are "no change". I would have expected at least a small change, but the main motivation for this change was to move away from enums and const enums in favor of normal constants and literal types, which eliminates a dependency on increasingly non-standard compilation infrastructure.

So no news is good news.

NullVoxPopuli commented 2 weeks ago

No perf difference on either of our local tests -- CI thinks improvement, so we'll merge.