perf(semantic): allocate symbol data in Allocator

[Boshen]

No description provided.

[Boshen]

• perf(semantic): allocate symbol data in Allocator #8012 👈 (View in Graphite)
• main

---

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• 0-merge - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

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

[codspeed-hq]

CodSpeed Performance Report

Merging #8012 will improve performances by 4.95%

_{Comparing 12-19-refactor_semantic_allocate_data_in_allocator (414e828) with main (8547e02)}

Summary

⚡ 3 improvements
✅ 26 untouched benchmarks

Benchmarks breakdown

	Benchmark	main	12-19-refactor_semantic_allocate_data_in_allocator	Change
⚡	semantic[antd.js]	111.4 ms	106.2 ms	+4.95%
⚡	semantic[checker.ts]	67.5 ms	65.3 ms	+3.42%
⚡	semantic[pdf.mjs]	18.2 ms	17.5 ms	+4.44%

[Boshen]

Merge activity

• Dec 20, 9:13 AM EST: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Dec 20, 9:13 AM EST: A user added this pull request to the Graphite merge queue.
• Dec 20, 9:17 AM EST: A user merged this pull request with the Graphite merge queue.

[overlookmotel]

@Boshen I think implementing Send on Allocator is perfectly safe. bumpalo::Bump implements Send, if I remember correctly.

But implementing Sync is definitely not safe, and likely a footgun (aka recipe for disaster).

I imagine Send is all that Rolldown needs?

[Boshen]

@Boshen I think implementing Send on Allocator is perfectly safe. bumpalo::Bump implements Send, if I remember correctly.

But implementing Sync is definitely not safe, and likely a footgun (aka recipe for disaster).

I imagine Send is all that Rolldown needs?

error[E0277]: `Cell<NonNull<bumpalo::ChunkFooter>>` cannot be shared between threads safely
   --> crates/rolldown_plugin/src/plugin_context.rs:127:13
    |
127 |       .await?;
    |             ^ `Cell<NonNull<bumpalo::ChunkFooter>>` cannot be shared between threads safely
    |
    = help: within `oxc_allocator::Allocator`, the trait `Sync` is not implemented for `Cell<NonNull<bumpalo::ChunkFooter>>`, which is required by `Result<(), anyhow::Error>: FromResidual<Result<Infallible, tokio::sync::mpsc::error::SendError<ModuleLoaderMsg>>>`
    = note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock`
    = help: the trait `FromResidual<Result<Infallible, E>>` is implemented for `Result<T, F>`
note: required because it appears within the type `bumpalo::Bump`
   --> /Users/boshen/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bumpalo-3.16.0/src/lib.rs:292:12
    |
292 | pub struct Bump {
    |            ^^^^
note: required because it appears within the type `oxc_allocator::Allocator`
   --> /Users/boshen/oxc/oxc/crates/oxc_allocator/src/lib.rs:71:12
    |
71  | pub struct Allocator {
    |            ^^^^^^^^^
    = note: required for `UnsafeSelfCell<SymbolTableCell, Allocator, SymbolTableInner<'static>>` to implement `Sync`
note: required because it appears within the type `oxc_semantic::symbol::SymbolTableCell`
   --> /Users/boshen/oxc/oxc/crates/oxc_semantic/src/symbol.rs:60:12
    |
60  |     struct SymbolTableCell {
    |            ^^^^^^^^^^^^^^^
note: required because it appears within the type `oxc_semantic::symbol::SymbolTable`
   --> /Users/boshen/oxc/oxc/crates/oxc_semantic/src/symbol.rs:21:12
    |
21  | pub struct SymbolTable {
    |            ^^^^^^^^^^^
note: required because it appears within the type `SymbolRefDbForModule`
   --> /Users/boshen/github/rolldown/crates/rolldown_common/src/types/symbol_ref_db.rs:37:12
    |
37  | pub struct SymbolRefDbForModule {
    |            ^^^^^^^^^^^^^^^^^^^^
note: required because it appears within the type `RuntimeModuleTaskResult`
   --> /Users/boshen/github/rolldown/crates/rolldown_common/src/module_loader/runtime_task_result.rs:8:12
    |
8   | pub struct RuntimeModuleTaskResult {
    |            ^^^^^^^^^^^^^^^^^^^^^^^
note: required because it appears within the type `ModuleLoaderMsg`
   --> /Users/boshen/github/rolldown/crates/rolldown_common/src/module_loader/mod.rs:11:10
    |
11  | pub enum ModuleLoaderMsg {
    |          ^^^^^^^^^^^^^^^
note: required because it appears within the type `tokio::sync::mpsc::error::SendError<ModuleLoaderMsg>`
   --> /Users/boshen/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.42.0/src/sync/mpsc/error.rs:8:12
    |
8   | pub struct SendError<T>(pub T);
    |            ^^^^^^^^^
    = note: required for `anyhow::Error` to implement `From<tokio::sync::mpsc::error::SendError<ModuleLoaderMsg>>`
    = note: required for `Result<(), anyhow::Error>` to implement `FromResidual<Result<Infallible, tokio::sync::mpsc::error::SendError<ModuleLoaderMsg>>>`
    = note: the full name for the type has been written to '/Users/boshen/github/rolldown/target/debug/deps/rolldown_plugin-966d208a591aeead.long-type-4702483371421541226.txt'
    = note: consider using `--verbose` to print the full type name to the console

[overlookmotel]

OK, well it's asking for trouble in my opinion! Bump is not thread-safe. You can move it from one thread to another no problem. But you can't safely share it between threads.