-
-
Notifications
You must be signed in to change notification settings - Fork 177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Immutable cache array #640
Conversation
@@ -54,7 +46,6 @@ Iterator.prototype._next = function (callback) { | |||
} | |||
|
|||
Iterator.prototype._end = function (callback) { | |||
delete this.cache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see tests failed on checking the cache size after calling .end()
, is there a particular reason to manually delete the cache array here?
AFAIK, deleting properties of a class is not a recommended practice since it might affect V8's hidden class.
Was working on that but I fell into a rabbit hole 😄 Climbing out! |
Climbed out. No actual benchmark written yet, only goodies: |
@vweevers you're on a roll! |
My little benchmark script shows these changes just made things slower 😢: #638 (comment) . I'm closing this PR now but feel free to reopen. |
Implemented two minor changes that might give us a performance gain:
.pop()
operations in JS land, leave thecache
array immutable.I haven't run the benchmarks yet, @vweevers is there any benchmark script suitable for this one?
Note: I forked this brach from remove-fast-future, I'll rebase after #638 gets merged.