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

feat: Node.js buffer built-in module implementation #1026

Merged
merged 11 commits into from
Jun 18, 2024
Merged

Conversation

darvld
Copy link
Member

@darvld darvld commented Jun 16, 2024

Draft Powered by Pull Request Badge

Summary

Adds support for the buffer built-in module from the Node.js API, with a few caveats:

  • Blob.arrayBuffer() is not yet functional since we can't create ArrayBuffer objects from the host (it is an internal GraalJS type).
  • Blob.stream() does not work since the ReadableStream type is not implemented yet.
  • buffer.isUtf8 and buffer.isAscii use a non-vectorized implementation that may be noticeably slower than in Node.js for large payloads.
  • buffer.transcode uses a very simple implementation that will degrade performance when used with large payloads.

Note: the module APIs will probably break in native mode due to the lack of proxy implementations, this will be fixed before the PR is ready to merge.

@darvld darvld self-assigned this Jun 16, 2024
@darvld darvld added module:graalvm Modules, changes, and issues relating to GraalVM 🚧 WIP Works-in-progress. Blocks merge lang:javascript Issues relating to JavaScript api:node Node API and stdlib labels Jun 16, 2024
Copy link

codecov bot commented Jun 17, 2024

Codecov Report

Attention: Patch coverage is 57.51634% with 65 lines in your changes missing coverage. Please review.

Project coverage is 55.10%. Comparing base (1d6d349) to head (6755937).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1026      +/-   ##
==========================================
+ Coverage   54.62%   55.10%   +0.47%     
==========================================
  Files         320      323       +3     
  Lines       10159    10263     +104     
  Branches     1760     1801      +41     
==========================================
+ Hits         5549     5655     +106     
+ Misses       4078     4042      -36     
- Partials      532      566      +34     
Flag Coverage Δ
gradle 55.10% <57.51%> (+0.47%) ⬆️
jvm 55.10% <57.51%> (+0.47%) ⬆️
lib 55.10% <57.51%> (+0.47%) ⬆️
plugin 55.10% <57.51%> (+0.47%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
.../elide/runtime/intrinsics/js/node/buffer/Buffer.kt 0.00% <ø> (-3.58%) ⬇️
...tlin/elide/runtime/intrinsics/js/node/BufferAPI.kt 0.00% <0.00%> (ø)
...lide/runtime/gvm/internals/node/buffer/NodeFile.kt 86.66% <86.66%> (ø)
...lide/runtime/gvm/internals/node/buffer/NodeBlob.kt 56.45% <56.45%> (ø)
...de/runtime/gvm/internals/node/buffer/NodeBuffer.kt 46.87% <45.00%> (+20.78%) ⬆️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d6d349...6755937. Read the comment docs.

@darvld darvld marked this pull request as ready for review June 17, 2024 04:41
darvld added 7 commits June 17, 2024 00:42
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
@darvld darvld force-pushed the feat/node-buffer branch from 7085d02 to 36cffc6 Compare June 17, 2024 04:42
darvld added 4 commits June 17, 2024 08:14
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
@sgammon sgammon merged commit 2342926 into main Jun 18, 2024
22 checks passed
@sgammon sgammon deleted the feat/node-buffer branch June 18, 2024 00:26
This was referenced Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api:node Node API and stdlib lang:javascript Issues relating to JavaScript module:graalvm Modules, changes, and issues relating to GraalVM 🚧 WIP Works-in-progress. Blocks merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants