Open
Bug 1505572
Opened 6 years ago
Updated 3 months ago
get*State should return an object instead of a serialized string
Categories
(Firefox :: Session Restore, enhancement, P2)
Firefox
Session Restore
Tracking
()
People
(Reporter: Gijs, Unassigned)
References
(Blocks 1 open bug)
Details
(Keywords: perf:resource-use, perf:responsiveness)
All the non-sessionstore consumers of these methods immediately parse the JSON back out again. This is a waste of CPU cycles (encoding and then immediately decoding again). We should just expose a non-JSON API.
Mike, do you know why these APIs return JSON at all? Is it just for IPC purposes or something? Or to avoid consumers changing the values when we pass object references, or something else?
Flags: needinfo?(mdeboer)
Comment 1•6 years ago
|
||
(In reply to :Gijs (he/him) from comment #0)
> Or to avoid consumers changing the values when we
> pass object references, or something else?
This. At the moment I can't really think of a way to do this other than using `Cu.cloneInto` to make a structured clone (copy) of the data.
Flags: needinfo?(mdeboer)
Reporter | ||
Comment 2•6 years ago
|
||
(In reply to Mike de Boer [:mikedeboer] from comment #1)
> At the moment I can't really think of a way to do this other than
> using `Cu.cloneInto` to make a structured clone (copy) of the data.
Yes, but (as I'm sure you know) that should still be measurably faster than doing that via JSON.parse(JSON.stringify()) :)
Comment 3•6 years ago
|
||
Indeed, so I'd be very happy to see that change. I can work on that somewhere in the future, but perhaps else can find the cycles before I can...? ;-)
Updated•6 years ago
|
Whiteboard: [fxperf] → [fxperf:p2]
Updated•6 years ago
|
Whiteboard: [fxperf:p2] → [fxperf:p3]
Updated•2 years ago
|
Severity: normal → S3
Comment 4•1 year ago
|
||
We also have structuredClone now.
Performance Impact: --- → low
Keywords: perf:resource-use,
perf:responsiveness
Whiteboard: [fxperf:p3]
Reporter | ||
Comment 5•1 year ago
|
||
I think this might be WFM? Mark did a bunch of heroic work around this, IIRC.
Flags: needinfo?(standard8)
Comment 6•1 year ago
|
||
(In reply to Mike Conley (:mconley) (:⚙️) from comment #4)
We also have structuredClone now.
The bugs we've already fixed here, switched to using Cu.cloneInto
, since that's what was already being used in the non-string cases. I'm not sure which is better between the two for this case.
(In reply to :Gijs (he/him) from comment #5)
I think this might be WFM? Mark did a bunch of heroic work around this, IIRC.
There's still two functions to do:
- getBrowserState - this doesn't appear to have any uses outside tests, but lots of those tests do JSON.parse straight away...
- getTabState - there's a couple of uses here which would avoid the stringify -> parse cycle when dealing with lazy tabs in the re-opening container or swapping with a different browser. Again, there's also lots of tests that would benefit as well.
Flags: needinfo?(standard8)
You need to log in
before you can comment on or make changes to this bug.
Description
•