Skip to content

feat(napi): impl ReadableStream and AsyncGenerator#2418

Merged
Brooooooklyn merged 9 commits into
mainfrom
skip-node18-buggy-test
Jan 3, 2025
Merged

feat(napi): impl ReadableStream and AsyncGenerator#2418
Brooooooklyn merged 9 commits into
mainfrom
skip-node18-buggy-test

Conversation

@Brooooooklyn
Copy link
Copy Markdown
Member

@Brooooooklyn Brooooooklyn commented Jan 3, 2025

No description provided.

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Details
Benchmark suite Current: ed570b1 Previous: aad71cd Ratio
noop#napi-rs 86357933 ops/sec (±0.13%) 85541112 ops/sec (±0.56%) 0.99
noop#JavaScript 817466080 ops/sec (±0.06%) 813918006 ops/sec (±0.17%) 1.00
Plus number#napi-rs 22521144 ops/sec (±0.98%) 23002171 ops/sec (±0.3%) 1.02
Plus number#JavaScript 815716162 ops/sec (±0.1%) 812599279 ops/sec (±0.17%) 1.00
Create buffer#napi-rs 672825 ops/sec (±11.79%) 636198 ops/sec (±12.5%) 0.95
Create buffer#JavaScript 3423305 ops/sec (±2.46%) 3376417 ops/sec (±5.22%) 0.99
createArray#createArrayJson 53921 ops/sec (±0.17%) 53502 ops/sec (±0.64%) 0.99
createArray#create array for loop 10287 ops/sec (±0.45%) 10016 ops/sec (±1.05%) 0.97
createArray#create array with serde trait 10029 ops/sec (±0.33%) 10116 ops/sec (±0.73%) 1.01
getArrayFromJs#get array from json string 23650 ops/sec (±0.69%) 23824 ops/sec (±0.57%) 1.01
getArrayFromJs#get array from serde 12986 ops/sec (±0.64%) 13451 ops/sec (±0.52%) 1.04
getArrayFromJs#get array with for loop 15811 ops/sec (±0.4%) 15801 ops/sec (±0.58%) 1.00
Get Set property#Get Set from native#u32 583429 ops/sec (±12.44%) 542655 ops/sec (±11.45%) 0.93
Get Set property#Get Set from JavaScript#u32 548883 ops/sec (±2.12%) 540008 ops/sec (±2.16%) 0.98
Get Set property#Get Set from native#string 561945 ops/sec (±10.65%) 564867 ops/sec (±11.09%) 1.01
Get Set property#Get Set from JavaScript#string 527608 ops/sec (±1.77%) 529738 ops/sec (±1.72%) 1.00
Async task#spawn task 26071 ops/sec (±0.6%) 25900 ops/sec (±0.81%) 0.99
Async task#ThreadSafeFunction 9474 ops/sec (±1.92%) 9525 ops/sec (±0.62%) 1.01
Async task#Tokio future to Promise 32356 ops/sec (±1.05%) 32154 ops/sec (±1.19%) 0.99
Query#query * 100 3557 ops/sec (±0.74%) 3487 ops/sec (±0.88%) 0.98
Query#query * 1 27391 ops/sec (±0.76%) 26932 ops/sec (±0.9%) 0.98

This comment was automatically generated by workflow using github-action-benchmark.

fn next(
&mut self,
value: Option<Self::Next>,
) -> impl Future<Output = crate::Result<Option<Self::Yield>>> + Send + 'static;
Copy link
Copy Markdown

@lbeschastny lbeschastny Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just played with this new AsyncGenerator a little bit and I noticed that using 'static restricts us from holding a reference to self from the returned Future.

This may create some inconveniences for people trying to implement this trait.

Copy link
Copy Markdown

@lbeschastny lbeschastny Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I assume napi(async_iterator) will be implemented as well once AsyncGenerator is stabilised, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just played with this new AsyncGenerator a little bit and I noticed that using 'static restricts us from holding a reference to self from the returned Future.

This may create some inconveniences for people trying to implement this trait

Hmmm, maybe it's due to the limition of the tokio

Also, I assume napi(async_iterator) will be implemented as well once AsyncGenerator is stabilised, right?

Yes

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.

2 participants