-
Notifications
You must be signed in to change notification settings - Fork 808
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
http: add bindings to GET / #901
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #901 +/- ##
==========================================
+ Coverage 61.72% 61.72% +<.01%
==========================================
Files 155 155
Lines 26058 26059 +1
==========================================
+ Hits 16085 16086 +1
Misses 9973 9973
Continue to review full report at Codecov.
|
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 still need this after #729 ?
@@ -161,6 +162,10 @@ class HTTP extends Server { | |||
adjusted: this.network.now(), | |||
offset: this.network.time.offset | |||
}, | |||
bindings: { | |||
sha256: sha256.native === 2 ? 'native' : 'js', | |||
secp256k1: secp256k1.native === 2 ? 'native' : 'js' |
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.
Couldn't there also be the nodejs backend?
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.
Couldn't there also be the nodejs backend?
Good point. I was thinking JavaScript is JavaScript when I wrote this. From the last time that I looked at the code, the main difference between the node backend and the js backend was the usage of BigInt
in the node backend but browsers also natively support BigInt
these days. Ideally, there would be an integer <--> string mapping of these values in bcrypto
ThoughtsWas thinking about the viability of some sort of
node.js has recently added
It looks like other node.js developers have attempted to build some tools that can get in depth stats of the event loop, but based on their conversation it isn't easy and there isn't much reliable open source code doing this. Its done by writing some native code that plugs into
It seems like a closed source platform called N|Solid has implemented event loop metrics for their JavaScript runtime and they mentioned in one of the PRs above that it was not easy. Conclusion
|
Add
bindings
object toGET /
HTTP endpoint. Users sometime experience extremely slow IBD without realizing that it is using the Javascript backend. d553f0a might be enough to prevent this problem from happening again, opening this for discussion.