swc-project / swc

Rust-based platform for the Web
https://swc.rs
Apache License 2.0
31.26k stars 1.23k forks source link

refactor(es/compat): Use special span instead of passing `static_blocks_mark` #9725

Closed magic-akari closed 1 week ago

magic-akari commented 1 week ago

Description:

Let's make an assumption in the code: the use of dummy span as a private class field name is generated by us, it is there simply because it is necessary to maintain the order of side effect execution, its name is not referenced elsewhere, so we can safely remove its name.

This is a breaking change for Rust users.

BREAKING CHANGE:

Related issue (if exists):

changeset-bot[bot] commented 1 week ago

🦋 Changeset detected

Latest commit: 586b38b7ea441d661d547487dc787a500c1eb558

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codspeed-hq[bot] commented 1 week ago

CodSpeed Performance Report

Merging #9725 will degrade performances by 8.27%

Comparing magic-akari:refactor/static-blocks (586b38b) with main (aa0f784)

Summary

❌ 2 regressions ✅ 192 untouched benchmarks

:warning: Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main magic-akari:refactor/static-blocks Change
es/full/base/fixer 116.8 µs 127.4 µs -8.27%
es/visitor/base-perf/boxing_unboxed_clone 2.6 µs 2.6 µs -3.32%
magic-akari commented 1 week ago

Unfortunately, we have already used a similar trick here

https://github.com/swc-project/swc/blob/a417ff4d868b45a2157154e2334b8e1177c369e1/crates/swc_ecma_compat_es2015/src/classes/mod.rs#L631-L636

Or here

https://github.com/swc-project/swc/blob/49a8b3e756b4f99d2b958e68aa61e88e7d01d94e/crates/swc_ecma_compat_es2015/src/classes/constructor.rs#L23-L28

kdy1 commented 1 week ago

Hmm. Makes sense. I'll review this PR later today.

magic-akari commented 1 week ago

Or should we add another special span?

By the way: We added PURE_SP last time, but we didn't add the documentation properly.


I personally prefer to use dummy span directly.

kdy1 commented 1 week ago

I didn't think deeply enough while writing the DUMMY_SP hack because there was no Wasm plugin API, and I introduced static_block_mark because there's a custom plugin API at the moment of writing.

We may introduce another special flag in bytepos like SYNTHESIZED, one more for the Span type and use it, like you suggested.

Also, can you add a bit of documentation?