-
Notifications
You must be signed in to change notification settings - Fork 156
Add atob, btoa, structuredClone, queueMicrotask and performance #16
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
Comments
Doesn't the performance.now put us into a position where we need to start branching out to platform specific code? Is there a minimal implementation we could support and then decide to use it at context creation time with flags perhaps? |
Perhaps you can fetch counter from processor? Maybe RDTSC on X86 would help? That would be compiler specific which is not that bad imho. |
Nwm it needs TSC which is platform specific. That means it belongs to std runtime implementation. |
Node.js uses uv_hrtime() and that's basically a wrapper around clock_gettime(CLOCK_MONOTONIC) or QueryPerformanceCounter(). Seems okay to me to use that here as well. |
Yep I had that in mind too. Does also work on macOS and BSDs? I haven't checked the implementation in a while :-) If it's simple let's do it ! |
Even macOS has clock_gettime as of 10.12 so I don't think there are any blockers. |
Awesome! |
Yeah but clock_gettime is not everywhere just like QueryPerformanceCounter(). I suggest to not make performance or any other platform specific things to be part of the core library. Sort of extension to standard lib is more useful. With proper defines which allows you to opt-out of these things if needed. |
Why not? Implementing |
Where is it missing? Linux, macOS, (Free|Net|Open|Dragonfly)BSD, Solaris, Fuchsia, QNX and Emscripten all have it. |
Brain dump, since I've been thinking about these a bit the past week. Both atob/btoa and structuredClone depend on DOMException for error reporting, and if we are to do it we might as well make it compliant, so we should have DOMException I guess. I noticed no other engine provides structuredClone, it seems to always be implemented by the embedder for some reason. If we can do it, then why not? (IMHO). Last, atob/btoa require a base64 encoder/decoder, which we will also need to add for https://github.com/tc39/proposal-arraybuffer-base64 so I guess it ties it up nicely... |
I took a shot at implementing atob/btoa here: https://gist.github.com/graham/0f6f0f7de6c2839155c7f36ee7625588 It uses come c++ stdlib and an external library for printing, but I'll remove all of that before a create a pr. I'm a little confused about how I should go about testing this? I could write something in I don't know enough about how the arraybuffer work is going to know if this is helpful or a distraction. Actual Question: Assuming I'm going to make a pr to add atob and btoa; where would you like me to implement the tests to ensure it works correctly? Alternatively, if this is already in the works I can find another issue to focus on. |
Looks good! I'd add them to the global object by default, perhaps with the option to not do that with some helper JS_AddIntrinsicAtob for those who want to use the raw context. This way tests can be written in JS. Error handling is something we need to solve though. In the spec they can throw DOMException, so we likely need to implement that first or as part of this work. |
I've started with naive implementation, there is significant discussion about how to make this faster: https://stackoverflow.com/questions/342409/ If there is interest I rewrite with some of these tricks.
I took another shot at this: #1032 Removed all the c++ and tried to make it as much like the other modules as I could. My initial implementation is pretty basic, after completing it I found: https://stackoverflow.com/questions/342409/ as well as a couple other articles about how to make this more performant if there is interest. This is my first contribution so feel free to be extra picky if I missed a style or organization detail. (any feedback is welcome) |
Continuing from saghul/txiki.js#418:
To avoid duplication across embedders, implement atob, btoa, structuredClone1, queueMicrotask and performance as quickjs-ng C APIs and/or JS globals. We also have to add DOMException but that seems okay to me. We can make them opt-in when the JSContext is created.
1 TBD how to handle transferables
The text was updated successfully, but these errors were encountered: