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

chore(napi): Refactor async parsing functions to remove tokio dependency #4049

Merged

Conversation

cblh
Copy link
Contributor

@cblh cblh commented Jul 4, 2024

Resolves #4044

  • Added use napi::{bindgen_prelude::AsyncTask, Task} to handle async tasks without tokio.
  • Introduced ResolveTask struct implementing Task for asynchronous parsing.
  • Replaced parse_sync function implementation with parse_with_return to reuse code.
  • Refactored parse_async to use AsyncTask and ResolveTask for async parsing.
  • Removed tokio dependency by avoiding tokio::spawn and using AsyncTask for async operations.

This refactor enhances code readability and removes the dependency on tokio, streamlining the async task handling within the napi framework.

- Added `use napi::{bindgen_prelude::AsyncTask, Task}` to handle async tasks without tokio.
- Introduced `ResolveTask` struct implementing `Task` for asynchronous parsing.
- Replaced `parse_sync` function implementation with `parse_with_return` to reuse code.
- Refactored `parse_async` to use `AsyncTask` and `ResolveTask` for async parsing.
- Removed tokio dependency by avoiding `tokio::spawn` and using `AsyncTask` for async operations.

This refactor enhances code readability and removes the dependency on tokio, streamlining the async task handling within the napi framework.
Copy link

graphite-app bot commented Jul 4, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

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

Copy link

codspeed-hq bot commented Jul 4, 2024

CodSpeed Performance Report

Merging #4049 will not alter performance

Comparing cblh:feature/20240703chore_napi_remove_tokio (089dbad) with cblh:feature/20240703chore_napi_remove_tokio (41bd99b)

Summary

✅ 28 untouched benchmarks

@Boshen
Copy link
Member

Boshen commented Jul 4, 2024

tokio in Cargo.toml can be removed as well.

@cblh
Copy link
Contributor Author

cblh commented Jul 4, 2024

tokio in Cargo.toml can be removed as well.

Another part of the code also needs modification:

tokio::spawn(async move { module_lexer(source_text, options) }).await.unwrap()

cblh added 2 commits July 4, 2024 18:30
- Simplified parse_with_return function to eliminate unnecessary lifetime annotation and unnecessary borrowing of variables.
- Updated span.source_text usage to directly reference parameters instead of borrowing.
- Changed module_lexer function parameters from String and Option<ParserOptions> to &str and &ParserOptions, using unwrap_or_default for the options parameter.
- Updated module_lexer_sync function to call module_lexer with &source_text and &options.
- Introduced napi::{bindgen_prelude::AsyncTask, Task}, and implemented the Task trait for the ResolveTask struct.
- Added support for AsyncTask<ResolveTask> in the module_lexer_async function to handle asynchronous tasks.
@Boshen Boshen merged commit 38e7351 into oxc-project:main Jul 4, 2024
23 checks passed
@Boshen
Copy link
Member

Boshen commented Jul 4, 2024

Thank you!

@cblh cblh deleted the feature/20240703chore_napi_remove_tokio branch July 9, 2024 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove tokio from napi outputs
2 participants