Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use feature gate on parallelism #2959

Closed
hyf0 opened this issue Apr 13, 2024 · 4 comments
Closed

Use feature gate on parallelism #2959

hyf0 opened this issue Apr 13, 2024 · 4 comments
Assignees
Labels
C-enhancement Category - New feature or request

Comments

@hyf0
Copy link
Contributor

hyf0 commented Apr 13, 2024

Related:

Currernt oxc_sourcemap enable using rayon by default and it show improved performance said in #2825 (comment).

However, wasm target doesn't have a good support in rayon. While rayon claims it's ok in some issues, but it's still quite frustrating to debug wasm + rayon. It would be helped if we could disable rayon using feature gate.


Off the topic, Using feature gate on parallelism should be a principle that apply on all oxc crates that gonna exposed to users when introducing any parallelism in it.

@hyf0 hyf0 added the C-enhancement Category - New feature or request label Apr 13, 2024
@hyf0
Copy link
Contributor Author

hyf0 commented Apr 13, 2024

I also have some confusions on #2825 (comment). While it said it has improved the performance overall, but it doesn't seems to be a suitable strategy for every situation.

I don't know much about these two funcitons:

pub fn encode(sourcemap: &SourceMap) -> Result<String> {

But it seems that gonna be used in codegen. Then I realized these are gochas in logic

  • Rolldown is rendering modules, which means using oxc_codegen, in parallel already, so if this parallelism is enable by default , I'm not sure if it really improve performance in rendering modules while we are already apply parallelism in module level.

  • Rolldown will use oxc_sourcemap directly to manipulate the sourcemap in chunk level. Which means apply these functions on modules in chunk in serial.

I suspect that's where it really improve the performance since the benchmark case used in #2825 (comment), which is a huge single chunk.

If we have a case that generate so many mutiple small chunks, I wonder if it would be even a downside to the performance.

However, it still show pentation on the case said #2825 (comment). So using feature gate might not enough, since we need dynamicly decide to use which method depend on the input.

@Boshen
Copy link
Member

Boshen commented Apr 15, 2024

Let's either:

  • remove rayon from this case
  • add benchmark to indicate real world scenarios

@hyf0
Copy link
Contributor Author

hyf0 commented Apr 15, 2024

Let me try to find a way to benchmark this. On hold for now.

@Boshen Boshen assigned underfin and unassigned Boshen Apr 18, 2024
@Boshen Boshen assigned Boshen and unassigned underfin May 7, 2024
@Boshen
Copy link
Member

Boshen commented May 13, 2024

Next release:

sourcemap = ["oxc_sourcemap"]
sourcemap_concurrent = ["sourcemap", "oxc_sourcemap/concurrent"]

@Boshen Boshen closed this as completed May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category - New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants