-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
tls_wrap: proxy handle methods in prototype #1108
Conversation
Set proxied methods wrappers in `TLSWrap` prototype instead of doing it on every socket allocation. Should speed up things a bit and will certainly make heapsnapshot less verbose.
cc @iojs/collaborators too |
CI: good |
if (this._parent[name]) | ||
return this._parent[name].apply(this._parent, arguments); | ||
}; | ||
}); |
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.
do we care about the extra performance lost to creating a closure and referring to the call as this._parent[name]
instead of this._parent.<name>
?
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.
Nope, it shouldn't be critical here.
The change looks good. Just have a question. |
I see there is one TLS test failing on Windows but I'm not sure if it's related to this change... |
3 failures on windows are persistent and I don't believe they are related to this |
I checked previous runs and parallel/test-tls-over-http-tunnel seems to be failing consistently (and it's always the same 3 or 4 tests that fail.) LGTM then. |
Set proxied methods wrappers in `TLSWrap` prototype instead of doing it on every socket allocation. Should speed up things a bit and will certainly make heapsnapshot less verbose. PR-URL: #1108 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Trevor Norris <[email protected]>
Landed in 8431fc5, thank you everyone! |
Set proxied methods wrappers in
TLSWrap
prototype instead of doing iton every socket allocation. Should speed up things a bit and will
certainly make heapsnapshot less verbose.
cc @iojs/crypto