Famous / engine

MIT License
1.75k stars 250 forks source link

refact: Remove obsolete data clone #432

Closed alexanderGugel closed 9 years ago

alexanderGugel commented 9 years ago

In the current implementation a new typed array is being instantiated on every subData call using concat. This should be factored out by sending the data as a transferable ArrayBuffer through postMessage, resulting into a zero-copy transfer. For the time being, we can at least remove the obsolete cloning of the array using concat.

@michaelobriena Is there a case in which this.data is nested (I couldn't come up with one, but I'm not sure)? If that's the case the proposed solution wouldn't work.

michaelobriena commented 9 years ago

need to test this. The comment in the code you removed seemed to denote this was a must have. Will dig into it.

alexanderGugel commented 9 years ago

I'm assuming

Array.prototype.concat.apply(data, data)

was resulting into excessive growth of the call stack - therefore the chunks were introduced, but after all it's not necessary in the first place to clone the array since a new typed array is being created using data (which in turn results into the allocation of the underlying ArrayBuffer + DataView via the typed array).

alexanderGugel commented 9 years ago

@michaelobriena Did you figure out why this was a must have?

michaelobriena commented 9 years ago

Looks like you were right. Ready to Merge

alexanderGugel commented 9 years ago

landed as 782be3d