thlorenz / v8-perf

⏱️ Notes and resources related to v8 and thus Node.js performance
2.2k stars 112 forks source link

Advice about array sizes could be misleading #1

Closed petkaantonov closed 10 years ago

petkaantonov commented 10 years ago

The list at https://github.com/thlorenz/v8-perf/blob/master/data-types.md#fast-elements implies that fast elements have the characteristic of being smaller than 64k. But there is no such restriction, a single array or object can be any size (before causing oom error) and still have fast elements.

The considerations list at https://github.com/thlorenz/v8-perf/blob/master/data-types.md#considerations-1 says not to pre-allocate arrays with more than 64k elements, but this limit has always been 99999 and has never been changed (except when it was temporarily changed to 94999).

thlorenz commented 10 years ago

Interesting, I'd like to learn more about this. Could you link some resources or to the relevant code snippet in v8?

I gathered this information from this slide and this video around this time. So either the info there was wrong or I misinterpreted.

In any case would love to find more material about this topic and thanks for the feedback.

petkaantonov commented 10 years ago

I think best way is just to do this:

file.js

function HasFastElements(obj) {
  return %HasFastSmiElements(obj) ||
      %HasFastSmiOrObjectElements(obj) ||
      %HasFastObjectElements(obj) ||
      %HasFastDoubleElements(obj) ||
      %HasFastHoleyElements(obj);
}

var arr = [];
//10,000,000
var len = 10000000;

while(len--) {
  arr.push(len);
}

console.log("Array has fast elements: " + HasFastElements(arr));

var a = new Array(99999);
var b = new Array(100000);

// this was already proven by the link to v8 source, but let's verify it anyway
console.log("a has fast elements: " + HasFastElements(a));
console.log("b has fast elements: " + HasFastElements(b));

Output:

$ node --allow-natives-syntax file.js
Array has fast elements: true
a has fast elements: true
b has fast elements: false
thlorenz commented 10 years ago

I get it now, thanks for pointing this out.

single array or object can be any size (before causing oom error) and still have fast elements

Again just quoting from this info, so I'm pretty sure this was correct at least at the time when the talk was given.

I can imagine however that things changed and from the source you linked it looks like it has. Actually from the diff it looks like it changed from 5,000 elements to 100,000 - so no 64k anywhere.

So I don't agree with any size, since a limit is clearly defined in the code you linked, but agree that the 64k may be incorrect. However also keep in mind that this change landed in 3.26.30 which not many are using - specifically nodejs still uses an older v8 version (even node v0.11).

Gonna try to get some chrome devs to give the final verdict here via the twitters and will update content accordingly.

thlorenz commented 10 years ago

I actually seem to be wrong about

this change landed in 3.26.30 which not many are using

since your test passes with node v0.10.26 which uses v8 v3.14.5.9. I'm gonna add your test to this repo and will make it easy to run against different node versions.

thlorenz commented 10 years ago

@petkaantonov added your test here so we can run those simply via npm test against different node and thus v8 versions.

Needless to say that I'm very convinced now that you are correct :) and if I don't hear from anyone else over the course of this weekend I'll go with that and update the notes.

Please feel free to add more corrections/tests. Thanks.

petkaantonov commented 10 years ago

Awesome idea to have "unit tests" for this

thlorenz commented 10 years ago

Yeah, that way we'll catch if things change with future v8 versions :)

I updated the document to correct it with your info. Again thanks for helping out in getting this right.