V8 4.4 to remove indexed properties via external data

[jeisinger]

Hey,

I'm about to remove SetIndexedPropertiesToExternalArrayData and related APIs from V8. I think there are some callsites of this API in io.js that would need to be updated.

You should use the standardized ArrayBuffers / ArrayBufferViews instead.

CL to remove them is here: https://codereview.chromium.org/1092923002

this will land in V8 4.4

[bnoordhuis]

I just saw the email on v8-dev and I've left a comment. I appreciate the heads up and I can't stress enough the impact that change will have on the io.js and node.js ecosystem.

[mscdex]

Yikes

[jeisinger]

I realize this is disruptive, but I don't see how it's fundamentally different from e.g. the MaybeLocal<> or Persistent<> changes that also force existing embedders to update.

anyway, as I mentioned on the CL, we'll discuss this internally. Will update this issue with the outcome.

[mscdex]

@bnoordhuis Doesn't this mean that it would no longer be possible to have Buffer?

[jeisinger]

the same functionality is provided by the ArrayBuffer API. The javascript objects will look exactly the same.

[petkaantonov]

It's not just compatibility but you can't implement something like SharedArrayBuffer (partial ownership transfers) with only ArrayBuffer api

[Fishrock123]

iirc ArrayBuffer APIs aren't able to completely replace Buffer?

See @petkaantonov's comment..

We'd need enough hooks to at least make a replacement...

[jeisinger]

All those use cases should be supported. If you have a concrete instance of
problem, I'm happy to help

On Fri, Apr 17, 2015, 5:38 PM Jeremiah Senkpiel [email protected]
wrote:

iirc ArrayBuffer APIs aren't able to completely replace Buffer? We'd need
enough (something) to at least make a replacement...

—
Reply to this email directly or view it on GitHub
#1451 (comment).

[meandmycode]

I feel if there aren't any blocking functionality issues then this is ok.

It seems more of an agenda issue to say making breaking changes is unacceptable. It won't break node as node lives on v8 from the past and they've said they won't update v8 often because of changes like this.

Whereas iojs was willing to take the hit for the better engine, I'm not sure you can always have your cake and eat it..

If iojs has an agenda to keep up with v8 then it also needs to point out that this can be disruptive to native modules.

[bmeck]

I think you can do transferable ArrayBuffers using ArrayBuffer::Externalize(), ArrayBuffer::Neuter(), then ArrayBuffer::New(). Or if you want shared mutation don't Neuter() after Externalize(). @jeisinger does that sound right?

[jeisinger]

You can just allocate the memory yourself using the ArrayBufferAllocator node already defines and pass that memory to ArrayBuffer::New()

I'd put the pointer to the underlying memory in one of the internal fields of the arraybuffer (via SetAlignedPointerInInternalField) because the ArrayBuffer::Externalize API only gives you the pointer once...

If there are additional APIs you'd need on ArrayBuffer, ArrayBufferView, or the typed Arrays, we can certainly add them there.

[jeisinger]

basically, what will be dropped is the ability to add external data to any object. Instead, the object needs to inherit from one of the typed array types (ArrayBufferView) or DataView.

All other functionality will stay - if something is not accessible via the typed array API this can be easily fixed.

[vkurchatkin]

@jeisinger so it will be possible to access ArrayBuffer data without copying/externalizing?

[jeisinger]

Note that externalizing doesn't copy (unless you had a small typed array allocated on the V8 heap - you can disable this by setting --typed_array_max_size_in_heap=0).

Whether an array buffer is externalized or not just decides who has to free the memory. If you'd have V8 manage the memory, I can add a method ArrayBuffer::GetContents that returns a pointer to the data without externalizing it. Note that this is a bit of a foot-gun as the pointer will stop being valid as soon as V8 gc'd the object.

[vkurchatkin]

@jeisinger it's not hard to prevent object from being gc'd with C++ API, the problem with externalizing is that you can't unexternalize when you are done with the buffer

[jeisinger]

something like this should fix this: https://codereview.chromium.org/1095083002

[trevnorris]

@jeisinger This will at minimum require removing an entire API. There are also a couple other things I'd like to ask:

• Is it possible to resize an ArrayBuffer after it's been created?
• IIRC small ArrayBuffers are allocated in the V8 heap, so will GetContents() work differently in those cases? Meaning, we'd need to make sure the location of that memory doesn't change while working with the data.

[jeisinger]

it's not possible to resize an array buffer, but you can create arbitrary views on that buffer.
on-heap buffers can be disabled by passing --typed_array_max_size_in_heap=0

[trevnorris]

@indutny have an idea of what kind of overhead to expect from needing to always determine the correct number of bytes being written instead of being able to realloc() afterwards?

@jeisinger I'm glad to see there's a cli option, but IMHO needing to forcefully set that at application start isn't a solution to the issue.

@thlorenz has told me it's possible to compile in V8 options. Guess we'd just have to use that.

[jeisinger]

node.cc has several calls to SetFlagsFromString - why not just put it there?

[jeisinger]

actually, I think it might be possible to resize array buffers, let me check

[jeisinger]

ES7 defines ArrayBuffer.transfer() but it's not yet implemented in V8 :-/

[trevnorris]

@jeisinger Thanks for taking the time to look into it. :)

[domenic]

Streams will want ArrayBuffer.transfer-like functionality (not necessarily publicly exposed, but at least in the engine) so I hope it shows up sooner or later :)

[jeisinger]

after looking more into it, ArrayBuffer.transfer will not let you change the capacity of one object. It still creates a new ArrayBuffer, it just allows the underlying implementation to use realloc() instead of just copying the contents.

So while we might add that API anyway, it won't solve your problem that you want to change the capacity of a given object :-/

[chrisdickinson]

Just a note: it might be possible to base Buffer off of Uint8Array, a la @feross' work with buffer in browserify.

[trevnorris]

@chrisdickinson That is the plan. I'm working on test implementations ATM using latest V8 4.4. The main breakage comes that all native modules that use Buffer will break. I'm seeing what can be done to mitigate that. The patch https://codereview.chromium.org/1095083002 from @jeisinger will help.

[trevnorris]

I was hoping we could get it closer to the normal ES6 class syntax performance time:

'use strict';
const ITER = 1e7;

class Nuffer extends Uint8Array {
  constructor(n) {
    super(n);
  }
}

var t = process.hrtime();
for (var i = 0; i < ITER; i++)
  new Nuffer(1);
t = process.hrtime(t);
t = t[0] * 1e9 + t[1];
console.log((t / ITER).toFixed(1) + ' ns/op');

Runs in ~245 ns/op. Unfortunately once the size of the allocation increases we're seriously hosed because of the need to zero-fill.

As per usual I have a very nasty way to get around the performance issue for allocating Buffers from JS, but I'd very much prefer not to abuse the V8 API that way. It also doesn't fix the performance issue of creating a new Buffer around an existing void*.

[jeisinger]

I think it should be possible to get the performance of those three C++
calls down to the js performance, however, that's probably going to be a
4.5 thing.

On Tue, May 5, 2015, 11:27 PM Trevor Norris [email protected]
wrote:

I was hoping we could get it closer to the normal ES6 class syntax
performance time:

'use strict';const ITER = 1e7;
class Nuffer extends Uint8Array {
constructor(n) {
super(n);
}
}
var t = process.hrtime();for (var i = 0; i < ITER; i++)
new Nuffer(1);
t = process.hrtime(t);
t = t[0] * 1e9 + t[1];console.log((t / ITER).toFixed(1) + ' ns/op');

Runs in ~245 ns/op. Unfortunately once the size of the allocation
increases we're seriously hosed because of the need to zero-fill.

As per usual I have a very nasty way to get around the performance issue
for allocating Buffers from JS, but I'd very much prefer not to abuse the
V8 API that way. It also doesn't fix the performance issue of creating a
new Buffer around an existing void*.

—
Reply to this email directly or view it on GitHub
#1451 (comment).

[kkoopa]

@domenic I queried npmjs regarding packages that depend on NAN indirectly or directly. The result was 60453 unique packages. npm currently lists 146600 packages. Thus, 60453/146600 = 41 % of packages in npm depend on NAN in some way, and thereby also depend on native modules. So, while it may be technically true that most users don't depend on native modules, a significant chunk do depend on them.

[trevnorris]

@jeisinger I've noticed that calling super() from constructor() doesn't allow it to every be optimized. Any chance that could be added to the 4.5 performance roadmap as well? I'll take a look under-the-hook, but my V8 chops probably aren't good enough to make a CL.

[jeisinger]

Yes, many of the newly added ES6 features aren't yet fully optimized. I don't know what the roadmap for specific features is.

[rvagg]

removing tsc-agenda label from this, @trevnorris please feel free to put a label back on one of the related issues if you'd like to discuss the topic further

[bnoordhuis]

I think this can be closed, the relevant core work was done in #1825 and that landed in 65cd82a...f72ecc4.

[trevnorris]

@jeisinger Here was my solution to allowing creating the Uint8Array from JS but still not filling it: 74178a5

Horrible abuse of the API, but is working great.

[jeisinger]

You'll lose the optimizations for length and other fields that way, no
(because the prototype is modified)?

On Wed, Sep 16, 2015, 1:38 AM Trevor Norris [email protected]
wrote:

@jeisinger https://github.com/jeisinger Here was my solution to
allowing creating the Uint8Array from JS but still not filling it: 74178a5
74178a5

Horrible abuse of the API, but is working great.

—
Reply to this email directly or view it on GitHub
#1451 (comment).

[trevnorris]

/cc @indutny have info for @jeisinger's question?

[trevnorris]

@jeisinger Wouldn't we have lost these anyway by using v8::Object::SetPrototype()?

[indutny]

@jeisinger I think I just fixed that in v8

[jeisinger]

ah, k