Skip to content
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

Add atob, btoa, structuredClone, queueMicrotask and performance #16

Open
3 of 6 tasks
bnoordhuis opened this issue Nov 5, 2023 · 11 comments
Open
3 of 6 tasks

Add atob, btoa, structuredClone, queueMicrotask and performance #16

bnoordhuis opened this issue Nov 5, 2023 · 11 comments
Labels
enhancement New feature or request

Comments

@bnoordhuis
Copy link
Contributor

bnoordhuis commented Nov 5, 2023

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

@bnoordhuis bnoordhuis added the enhancement New feature or request label Nov 5, 2023
@saghul
Copy link
Contributor

saghul commented Nov 5, 2023

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?

@M0n7y5
Copy link

M0n7y5 commented Nov 5, 2023

Perhaps you can fetch counter from processor? Maybe RDTSC on X86 would help? That would be compiler specific which is not that bad imho.

@M0n7y5
Copy link

M0n7y5 commented Nov 5, 2023

Nwm it needs TSC which is platform specific. That means it belongs to std runtime implementation.

@bnoordhuis
Copy link
Contributor Author

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.

@saghul
Copy link
Contributor

saghul commented Nov 5, 2023

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 !

@bnoordhuis
Copy link
Contributor Author

Even macOS has clock_gettime as of 10.12 so I don't think there are any blockers.

@saghul
Copy link
Contributor

saghul commented Nov 6, 2023

Awesome!

@M0n7y5
Copy link

M0n7y5 commented Nov 6, 2023

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.

@saghul
Copy link
Contributor

saghul commented Nov 6, 2023

Why not? Implementing Atomics already requires some if-defing around, and if all we need is to support 2 implementations of performance.now, that's a drop in a bucket compared to the size of quickjs.c.

@bnoordhuis
Copy link
Contributor Author

clock_gettime is not everywhere

Where is it missing? Linux, macOS, (Free|Net|Open|Dragonfly)BSD, Solaris, Fuchsia, QNX and Emscripten all have it.

saghul added a commit that referenced this issue Nov 17, 2023
saghul added a commit that referenced this issue Nov 18, 2023
saghul added a commit that referenced this issue Nov 21, 2023
saghul added a commit that referenced this issue Nov 21, 2023
saghul added a commit that referenced this issue Nov 21, 2023
@saghul
Copy link
Contributor

saghul commented Mar 7, 2024

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants