Save/load structured clone data from JS shell

RESOLVED FIXED in Firefox 57

Status

()

P3
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: sfink, Assigned: sfink)

Tracking

(Blocks: 1 bug)

unspecified
mozilla57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

a year ago
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.
(Assignee)

Comment 1

a year ago
Created attachment 8908924 [details] [diff] [review]
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.
Attachment #8908924 - Flags: review?(jcoppeard)
(Assignee)

Comment 2

a year ago
Created attachment 8908925 [details] [diff] [review]
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.
Attachment #8908925 - Flags: review?(jcoppeard)
status-firefox57: --- → fix-optional
Priority: -- → P3
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.
Attachment #8908924 - Flags: review?(jcoppeard) → review+
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.
Attachment #8908925 - Flags: review?(jcoppeard) → review+
(Assignee)

Updated

a year ago
Blocks: 1400970
(Assignee)

Comment 5

a year ago
(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

a year ago
Pushed by sfink@mozilla.com:
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

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/383a2f567ceb
https://hg.mozilla.org/mozilla-central/rev/18af2ea42a1e
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox57: fix-optional → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.