Bug fixes for dsl2ml #1
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The original polyfill had a bug that made the transfer happen incorrectly in Firefox under certain circumstances. This is the corrected polyfill.
|
Actually, I'll make this a separate PR. |
This can avoid an unnecessary allocation, since grow() unconditionally grows the buffer, and we don't need it to if the current buffer is already big enough for the jump. This is particularly relevant when freeDirective < 0, when the point of the jump is just to skip the remaining free space. The extra space that gets allocated in compileJump then gets immediately sliced out anyway, so you get the same end result after doing a lot more work.
|
Moved fix to separate PR: #2. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I tracked down the reason why FDQ in Firefox doesn't work in certain cases where it does in Chrome, such as for puzzles 2 and 3 in the sudoku example. The reason was a bug in the
ArrayBufferTransferPolypolyfill indsl2ml.js, which in certain cases clobbers data in Firefox.The first commit fixes that issue by copy & pasting the fixed implementation from the same place you copied the original. It looks like someone else noticed the bug in the MDN implementation and published the fix sometime after you grabbed it.
While I was debugging that issue, I noticed
compileJumpdoesn't need to callgrowhere when the jump doesn't go past the end ofmlBuffer. And in fact, thiscompileJumpinvocation (and supporting comments) doesn't(/don't) make sense ifcompileJumpalways allocates more memory unconditionally, as it currently does.The second commit adds a check before
compileJumpcallsgrowso it only callsgrowwhen necessary (and when it does callgrow, it only over-provisions by 10%, instead of 100% like it does now).