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

Potentially replace DefaultCompiler #573

Closed
na-- opened this issue Apr 5, 2018 · 2 comments
Closed

Potentially replace DefaultCompiler #573

na-- opened this issue Apr 5, 2018 · 2 comments
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 performance refactor

Comments

@na--
Copy link
Member

na-- commented Apr 5, 2018

This commit added dumb mutex-based synchronization the Compiler's Transform() method. That was necessary because the shared global DefaultCompiler instance was not thread safe.

We should benchmark this, but it would probably be better if instead of synchronizing a single compiler instance, we parse the babel JS source once (e.g. in an init() function), cache the result, remove DefaultCompiler and create new independent compiler instances wherever we want to use them.

@na-- na-- added the refactor label Apr 5, 2018
@na--
Copy link
Member Author

na-- commented May 30, 2018

This is a bit altered since the changes in #658 - now instead of a DefaultCompiler there's a "constructor" with a sync.Once() call to initialize something a singleton-like Compiler instance.

But the above issue still stands - we have to check if having a single instance that uses simple locking for the transpiling (as it currently is) is better than caching and sharing the result of goja's Compile()/CompileAST() for the babel source and using that to initialize totally separate compilers. This shouldn't make much difference for production k6 usage, but it could greatly affect the test execution speed once they are more parallel (#537).

@na-- na-- added the evaluation needed proposal needs to be validated or tested before fully implementing it in k6 label Oct 4, 2018
@na--
Copy link
Member Author

na-- commented Dec 14, 2021

I'm closing this because, as I mentioned in #2296 (comment), we plan to get rid of Babel completely eventually. It doesn't make sense to refactor it beforehand...

@na-- na-- closed this as completed Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 performance refactor
Projects
None yet
Development

No branches or pull requests

1 participant