[BUGFIX] Grow the Uint16Array if the page size overflows 1MB #798
Conversation
| private sizeCheck() { | ||
| this.pageSize++; | ||
| if (this.pageSize >= PAGE_SIZE) { | ||
| debugger; |
chadhietala
force-pushed
the
fix-array-growth
branch
from
ebf1593
to
abab453
Compare
| @@ -71,14 +74,26 @@ export class Heap implements CompileTimeHeap { | |||
| this.table = table; | |||
| this.offset = this.heap.length; | |||
| this.handle = handle; | |||
| this.pageSize = this.heap.length; | |||
There was a problem hiding this comment.
As this.pageSize keeps track of the used space in the last page of the heap, I think this will only work if serializedHeap.buffer is a multiple of PAGE_SIZE.
If in stead, we keep track of the free space, then this should work.
| this.table = []; | ||
| } | ||
| } | ||
|
|
||
| push(item: number): void { | ||
| this.heap[this.offset++] = item; | ||
| this.sizeCheck(); |
There was a problem hiding this comment.
The size should be checked before an element is put on the heap, otherwise a size check should also be executed in the constructor in case of a serializedHeap.
This has also the advantage that we don't need to grow the heap in case we end on a page boundary 😃
| private sizeCheck() { | ||
| this.pageSize++; | ||
| if (this.pageSize >= PAGE_SIZE) { | ||
| let heap = slice(this.heap, 0, this.offset); |
There was a problem hiding this comment.
I don't understand why the copy of the heap is necessary, is it not enough to keep a reference to the old heap in a local variable?
| @@ -181,6 +196,7 @@ export class Heap implements CompileTimeHeap { | |||
|
|
|||
| pushPlaceholder(valueFunc: () => number): void { | |||
| let address = this.offset++; | |||
| this.sizeCheck(); | |||
There was a problem hiding this comment.
As above, the size should be checked before an element is put on the heap.
chadhietala
force-pushed
the
fix-array-growth
branch
from
abab453
to
6b8fded
Compare
chadhietala
force-pushed
the
fix-array-growth
branch
from
f34a495
to
eddb932
Compare
|
Is this fix going to land in a point release of Ember 3.1 as a hotfix? |
|
Yes |
Fixes #797