970253 - We need a standard way to allocate an ArrayBuffer off the main thread.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Teller [:Yoric] - still alive but not very active]

Consider the following use case: C++ code needs to read large amounts of data off the main thread for use by a main thread JavaScript consumer (e.g. OS.File in bug 961665, DOM File() and FileReader() in bug 965956).

At the moment, we can code this in two manners:

A/ (bug 961665)
1. main thread sends request to worker thread;
2. worker thread reads data into temporary buffer;
3. main thread allocates ArrayBuffer and copies temporary buffer into ArrayBuffer.

Or B/ (bug 965956)
1. main thread determines size of data (causing main thread I/O) and allocates ArrayBuffer
2. worker thread reads data into ArrayBuffer.

Both solutions cause jank. We need a simple way to do avoid this:
- either allocate ArrayBuffer off the main thread for use on the main thread; or
- allow ArrayBuffer to adopt a pre-allocated C buffer.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Niko, I hear that you have some stuff for this.

Comment 2

[Boris Zbarsky [:bzbarsky]]

> - allow ArrayBuffer to adopt a pre-allocated C buffer.

People keep asking for this...  ;)

Comment 3

[David Teller [:Yoric] - still alive but not very active]

Pretty please?
Note that this feature would also come in quite handy for a possible js-ctypes rewrite.

Comment 4

[Steve Fink [:sfink] [:s:]]

(In reply to Boris Zbarsky [:bz] from comment #2)
> > - allow ArrayBuffer to adopt a pre-allocated C buffer.
> 
> People keep asking for this...  ;)

Being able to accept any random chunk of malloc'd memory would require a major change to object representation. That change would be worthwhile for other reasons, but nobody has the bandwidth to take it on right now.

(In reply to David Rajchenbach Teller [:Yoric] (please use "needinfo?") from comment #0)
> Both solutions cause jank. We need a simple way to do avoid this:
> - either allocate ArrayBuffer off the main thread for use on the main
> thread; or
> - allow ArrayBuffer to adopt a pre-allocated C buffer.

You can do something halfway between those already: JS_AllocateArrayBufferContents on the worker thread, then JS_NewArrayBufferWithContents on the main thread.

Or solution 3: allocate an ArrayBuffer on the worker thread, fill it in, and post it as a Transferable to the main thread. That's a single large allocation, no copying, just one garbage header created on the worker thread.

Comment 5

[David Teller [:Yoric] - still alive but not very active]

(In reply to Steve Fink [:sfink] from comment #4)
> You can do something halfway between those already:
> JS_AllocateArrayBufferContents on the worker thread, then
> JS_NewArrayBufferWithContents on the main thread.

Don't I need a JSContext for the worker to do that?

> Or solution 3: allocate an ArrayBuffer on the worker thread, fill it in, and
> post it as a Transferable to the main thread. That's a single large
> allocation, no copying, just one garbage header created on the worker thread.

Don't I need a JSContext for the worker to do that, too? Also, how do I post a Transferable in C++?

Comment 6

[Steve Fink [:sfink] [:s:]]

(In reply to David Rajchenbach Teller [:Yoric] (please use "needinfo?") from comment #5)
> (In reply to Steve Fink [:sfink] from comment #4)
> > You can do something halfway between those already:
> > JS_AllocateArrayBufferContents on the worker thread, then
> > JS_NewArrayBufferWithContents on the main thread.
> 
> Don't I need a JSContext for the worker to do that?

Oops, sorry. That's a bug in the header file. The 'cx' argument should be 'maybecx' -- you're allowed to pass in nullptr. (The only difference is that with a cx, it'll do memory accounting for GC pressure, and it'll report out of memory errors.)

> > Or solution 3: allocate an ArrayBuffer on the worker thread, fill it in, and
> > post it as a Transferable to the main thread. That's a single large
> > allocation, no copying, just one garbage header created on the worker thread.
> 
> Don't I need a JSContext for the worker to do that, too? Also, how do I post
> a Transferable in C++?

Yes, you do need a JSContext for that. I didn't realize you didn't have one.

I assumed there was a simple way to do postMessage from C++, but I don't actually know. I guess you could use JS_WriteStructuredClone/JS_ReadStructuredClone directly (passing in a 'transferable' array), but that's pretty awful. Actually, you should never do that if you can help it; at least use JSAutoStructuredCloneBuffer and its write() and read(). But you don't want to do that anyway. And it sounds like you don't have a runtime (or just a cx?) on the worker anyway, so this approach is a non-starter.

Comment 7

[Steve Fink [:sfink] [:s:]]

Attachment: cx param to JS_AllocateArrayBufferContents is optional

Comment 8

[Niko Matsakis [:nmatsakis]]

This is not really true. At one point we were discussing a similar scenario as a possible future use for typed objects -- kind of an efficient way to do a more customized version of structured clone, where you allocate a result buffer in the target's memory region and then let the user write into it directly -- but there are lot sof complications to be overcome and I'm not sure that would be really feasible. Anyway, it's not quite what you're looking for. (In fact, one of the challenges is overcoming precisely the problem I imagine you're having.)

(That said, with some refactoring, it seems plausible to support adopting a pre-allocated C buffer into an ArrayBuffer.)

I guess that bhackett's work on running various bits of the JS engine in other threads doesn't help?

Comment 9

[Boris Zbarsky [:bzbarsky]]

> Don't I need a JSContext for the worker to do that?

You should be able to get a JSContext if you need it, fwiw.  ThreadsafeAutoJSContext (over in nsCxPusher.h) will do the right thing in general.

> Also, how do I post a Transferable in C++?

A bit painfully; the buffer contents thing is simpler.

> it'll do memory accounting for GC pressure

We _want_ that for large allocations.  Skipping that part is sadfaces.

Comment 10

[Terrence Cole [:terrence]]

Comment on attachment 8373457 [details] [diff] [review]
cx param to JS_AllocateArrayBufferContents is optional

This patch appears empty?

Comment 11

[Steve Fink [:sfink] [:s:]]

Attachment: cx param to JS_AllocateArrayBufferContents is optional

The best patch is an empty patch. No added risk, easy to review.

Sorry; made the changes in the wrong directory.

Comment 12

[David Teller [:Yoric] - still alive but not very active]

(In reply to Boris Zbarsky [:bz] from comment #9)
> > Don't I need a JSContext for the worker to do that?
> 
> You should be able to get a JSContext if you need it, fwiw. 
> ThreadsafeAutoJSContext (over in nsCxPusher.h) will do the right thing in
> general.

That sounds interesting, but not quite enough documented for me to understand whether this serves my needs. 

What can I do with a ThreadsafeAutoJSContext? Allocate my ArrayBuffer object? Can I safely call JS_NewArrayBufferWithContents on the main thread with an ArrayBuffer allocated with another JSContext? How should I handle the ArrayBuffer object afterwards?

Comment 13

[Boris Zbarsky [:bzbarsky]]

> What can I do with a ThreadsafeAutoJSContext?

It gives you a reasonable JSContext for the current thread's JSRuntime, as long as the current thread is either main thread or a web worker thread.  Or is your worker thread some random C++ thread without an associated JSRuntime?

> Can I safely call JS_NewArrayBufferWithContents on the main thread with an ArrayBuffer
> allocated with another JSContext?

Yes, if you mean a void* allocated by JS_AllocateArrayBufferContents.

I guess if you're not on a web worker thread you can JS_AllocateArrayBufferContents with a null cx and then manually bump the malloc counter on the main thread when you JS_NewArrayBufferWithContents.

Comment 14

[Terrence Cole [:terrence]]

Comment on attachment 8373646 [details] [diff] [review]
cx param to JS_AllocateArrayBufferContents is optional

Review of attachment 8373646 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

Comment 15

[Steve Fink [:sfink] [:s:]]

(In reply to Boris Zbarsky [:bz] from comment #13)
> I guess if you're not on a web worker thread you can
> JS_AllocateArrayBufferContents with a null cx and then manually bump the
> malloc counter on the main thread when you JS_NewArrayBufferWithContents.

Looks like you can do the latter with:

extern JS_PUBLIC_API(void)
JS_updateMallocCounter(JSContext *cx, size_t nbytes);

Comment 16

[David Teller [:Yoric] - still alive but not very active]

(In reply to Boris Zbarsky [:bz] from comment #13)
> > What can I do with a ThreadsafeAutoJSContext?
> 
> It gives you a reasonable JSContext for the current thread's JSRuntime, as
> long as the current thread is either main thread or a web worker thread.  Or
> is your worker thread some random C++ thread without an associated JSRuntime?

In both scenarios, random C++ threads.

Comment 17

[Steve Fink [:sfink] [:s:]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b62d2430bcde

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b62d2430bcde

Comment 19

[Steve Fink [:sfink] [:s:]]

You can now hand any malloced buffer over to an ArrayBuffer. JS_NewArrayBufferWithContents will take any pointer that can be freed with free(), and in fact will do the accounting now when it takes over ownership.

Comment 20

[Steve Fink [:sfink] [:s:]]

Oops, meant to mark this FIXED.