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

Remove use of deprecated type v8::FastApiTypedArray #197

Merged

Conversation

gahaas
Copy link

@gahaas gahaas commented Jul 10, 2024

In the examples where v8::FastApiTypedArray was used, there is actually no advantage of using V8's fast API calls instead of regular API calls. Therefore this CL just removes the fast API call targets instead of adjusting them.

@victorgomes victorgomes merged commit d40ebc3 into v8:node-ci-2024-06-03 Jul 10, 2024
10 of 15 checks passed
@gahaas gahaas deleted the 2024-07-10_remove_typed_array branch July 10, 2024 12:19
pthier pushed a commit that referenced this pull request Jul 15, 2024
@ronag
Copy link

ronag commented Jul 29, 2024

@gahaas we have been benchmarking alternatives to v8::FastApiTypedArray and they are all significantly slower.¨

Refs: nodejs#54103
Refs: nodejs#54087

@gahaas
Copy link
Author

gahaas commented Jul 29, 2024

@ronag What exactly did you measure? If I remember correctly, the performance regression of removing v8::FastApiTypedArray was 6 CPU cycles per call. In a micro-benchmark that calls an empty API function, this regression was indeed clearly measurable, but as soon as I added reasonable workload, I could not measure a difference anymore.

@ronag
Copy link

ronag commented Jul 29, 2024

memmove 64 bytes.

@ronag
Copy link

ronag commented Jul 29, 2024

Pleas see the references

@gahaas
Copy link
Author

gahaas commented Jul 30, 2024

Thank you for making me aware of this issue. Could you please file a bug in the V8 bug tracker.

Please add information on how I can reproduce the issue locally.

Does node compile with PGO and LTO? I could imagine that the issue is that some API function calls get inlined in Chrome but not in node, as that would explain the different performance behavior.

@ronag
Copy link

ronag commented Jul 30, 2024

Does node compile with PGO and LTO? I could imagine that the issue is that some API function calls get inlined in Chrome but not in node, as that would explain the different performance behavior.

@targos Do you know someone that can answer this?

@ronag
Copy link

ronag commented Jul 30, 2024

victorgomes added a commit that referenced this pull request Aug 22, 2024
# Conflicts:
#	src/crypto/crypto_timing.cc
#	src/node_buffer.cc
#	src/node_external_reference.h
victorgomes pushed a commit that referenced this pull request Aug 22, 2024
pthier pushed a commit that referenced this pull request Oct 2, 2024
@ronag ronag mentioned this pull request Oct 21, 2024
7 tasks
victorgomes pushed a commit that referenced this pull request Nov 18, 2024
victorgomes pushed a commit that referenced this pull request Dec 18, 2024
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.

3 participants