1400466 - Save/load structured clone data from JS shell

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

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

In the hazard analysis, I have some intermediate text files that take quite a while to load, which really slows down the run-test-debug cycle. I tried reading/writing the data as JSON instead of parsing text files, but it really wasn't any faster. So I attempted to use structured cloning, but the current shell API is string-based, which turns out to not work well at all because the data gets mangled by being encoded on the input side, or truncated because it is passed to a C-style write function on the output side.

Comment 1

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

Attachment: Implement minimum necessary to allow saving and loading structured clone data

So I hacked the existing relevant routines to be able to do this, albeit clumsily. With this patch, you can do

  data = os.file.readFile("...", "binary");
  s = serialize(); // hack to get a clonebuffer
  s.clonebuffer = data.buffer;
  copy = deserialize(s);

I admit it would make more sense to add a more straightforward API, but the existing API is just wrong (mixing up string data and binary data).

I can file a followup to make a cleaner API in addition. Maybe removing the string-based stuff too.

Comment 2

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

Attachment: Cache the information loaded from src_comp.xdb into a structured clone buffer

Debugging utility that makes use of the structured cloning functionality. I'm still using text files for the "real" executions; once this properly lands, I'll probably switch over.

Comment 3

[Jon Coppeard (:jonco)]

Comment on attachment 8908924 [details] [diff] [review]
Implement minimum necessary to allow saving and loading structured clone data

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

This seems fine.  Please do file for cleanup/removing the string based API.

::: js/src/builtin/TestingFunctions.cpp
@@ +2422,5 @@
>  
>          Rooted<CloneBufferObject*> obj(cx, &args.thisv().toObject().as<CloneBufferObject>());
> +
> +        uint8_t* data = nullptr;
> +        auto freeData = mozilla::MakeScopeExit([&] { JS_free(cx, data); });

You can probably use UniquePtr for this.

Comment 4

[Jon Coppeard (:jonco)]

Comment on attachment 8908925 [details] [diff] [review]
Cache the information loaded from src_comp.xdb into a structured clone buffer

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

::: js/src/devtools/rootAnalysis/callgraph.js
@@ +216,5 @@
>          xdb.free_string(data);
>      }
>  }
> +
> +function loadTypesWithCache(type_xdb_filename, json_cache) {

This shouldn't be called json_cache if it doesn't contain JSON data.

Comment 5

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

(In reply to Jon Coppeard (:jonco) from comment #3)
> Comment on attachment 8908924 [details] [diff] [review]
> Implement minimum necessary to allow saving and loading structured clone data
> 
> Review of attachment 8908924 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This seems fine.  Please do file for cleanup/removing the string based API.

Bug 1400970

> ::: js/src/builtin/TestingFunctions.cpp
> @@ +2422,5 @@
> >  
> >          Rooted<CloneBufferObject*> obj(cx, &args.thisv().toObject().as<CloneBufferObject>());
> > +
> > +        uint8_t* data = nullptr;
> > +        auto freeData = mozilla::MakeScopeExit([&] { JS_free(cx, data); });
> 
> You can probably use UniquePtr for this.

Oh, duh. Good point.

> ::: js/src/devtools/rootAnalysis/callgraph.js
> > +
> > +function loadTypesWithCache(type_xdb_filename, json_cache) {
> 
> This shouldn't be called json_cache if it doesn't contain JSON data.

Heh. Yeah, I told myself to remember to change this name when I made the switch, but I never went back and did it.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/383a2f567ceb
Implement minimum necessary to allow saving and loading structured clone data, r=jonco
https://hg.mozilla.org/integration/mozilla-inbound/rev/18af2ea42a1e
Cache the information loaded from src_comp.xdb into a structured clone buffer, r=jonco

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/383a2f567ceb
https://hg.mozilla.org/mozilla-central/rev/18af2ea42a1e