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

V8 API: convert Local<Array> into a list of Local<Value> #12509

Closed
hashseed opened this issue Apr 19, 2017 · 7 comments
Closed

V8 API: convert Local<Array> into a list of Local<Value> #12509

hashseed opened this issue Apr 19, 2017 · 7 comments
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. v8 engine Issues and PRs related to the V8 dependency.

Comments

@hashseed
Copy link
Member

On last CTC/V8 meeting I think @trevnorris mentioned something to the effect of

Turning Local<Array> into a list of Local<Value> is slow. Possibly due to crossing the API boundary repeatedly. This is often done to extract function arguments from an array before calling the function. Exposing spread call to API would help a lot.

Could I solicit some examples in Node.js source? I'd like to understand the use case here to come up with the best solution. Two that I have in mind:

  • std::vector<Local<Value>> v8::Array::ToList(Local<Array>, int limit). This is more general purpose. We would be able to reduce the overhead of repeatedly crossing the API boundary and use V8's internal mechanisms to iterate an Array faster. It's debatable what to do with holes (prototype chain lookup?) and getters though.
  • v8::Function::SpreadCall(Local<Value> receiver, Local<Array> spread_args). From what I remember from the discussion, this is the primary use of turning an array into a list in the first place. V8 has optimizations in place to make spread calls very fast, so if the use case matches, I'd prefer this over the general purpose solution.

@fhinkel @psmarshall @bmeurer @trevnorris

@fhinkel
Copy link
Member

fhinkel commented Apr 19, 2017

@mscdex mscdex added v8 engine Issues and PRs related to the V8 dependency. c++ Issues and PRs that require attention from people who are familiar with C++. labels Apr 19, 2017
@addaleax
Copy link
Member

Just a quick note re using std::vector: #11048 (comment) … V8 should probably avoid using that as long as it’s also setting _GLIBCXX_DEBUG in debug mode, the two std::vector implementations aren’t ABI compatible.

@Trott
Copy link
Member

Trott commented Aug 15, 2017

Is there anything that can/should be added to this issue? Should it remain open? (I imagine "yes" but asking anyway.)

@bnoordhuis
Copy link
Member

cc @trevnorris

@addaleax
Copy link
Member

Got a bit of code for the first item in the issue together @ https://chromium-review.googlesource.com/c/622427

@jasnell
Copy link
Member

jasnell commented Aug 11, 2018

Should this remain open?

@fhinkel
Copy link
Member

fhinkel commented Oct 30, 2019

I'm doing a bit of issue clean up 🧹. I'm closing this due to inactivity. Please re-open if needed!

@fhinkel fhinkel closed this as completed Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

7 participants