Closed
Bug 668915
Opened 13 years ago
Closed 13 years ago
JSAutoStructuredCloneBuffer shouldn't require a 'cx'
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla8
People
(Reporter: luke, Assigned: bent.mozilla)
References
Details
(Whiteboard: [inbound])
Attachments
(1 file)
17.17 KB,
patch
|
jorendorff
:
review+
bent.mozilla
:
review+
|
Details | Diff | Splinter Review |
There are a few remaining off-main-thread JSAPI uses. The biggest remaining use is GetKeyPathValueFromStructuredData which does a JS_ReadStructuredClone. khuey has described a plan to store the key separately in the db entry so that it can be retrieved without deserializing the containing object. Another use is GetStructuredCloneDataFromStatement which seems to just need a 'cx' to call JSAutoStructuredCloneBuffer member functions (which itself just needs a 'cx' to call JS_malloc/JS_free). If that's all this is, we can just make change JSAutoStructuredCloneBuffer to not need a 'cx' (I can write that patch, just let me know if that's all this is). The last one is ClearStructuredCloneBuffer; its in the same boat as GetStructuredCloneDataFromStatement.
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → bent.mozilla
Status: NEW → ASSIGNED
Reporter | ||
Comment 1•13 years ago
|
||
This removes (the easy) 2 of the 3 cases. The patch just uses non-cx malloc which saves considerable GetSafeJSContext grossness in a few cases. This isn't in the JSAPI, so I out-of-lined JSAutoStructuredCloneBuffer members which I wouldn't think would matter for perf given that we just did a structured clone or are about to. Jason, could you check the js changes and Ben, could you check the dom changes?
Attachment #544840 -
Flags: review?(jorendorff)
Attachment #544840 -
Flags: review?(bent.mozilla)
Assignee | ||
Comment 2•13 years ago
|
||
Comment on attachment 544840 [details] [diff] [review] remove 'cx' needed for JSAutoStructuredCloneBuffer::clear/adopt Review of attachment 544840 [details] [diff] [review]: ----------------------------------------------------------------- r=me on the dom changes. ::: dom/indexedDB/IDBObjectStore.cpp @@ +840,5 @@ > NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR); > > + return aBuffer.copy(reinterpret_cast<const uint64 *>(data), dataLength) > + ? NS_OK > + : NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR; Nit: ? and : on previous lines. ::: js/src/jsapi.h @@ +3364,5 @@ > void *closure); > > #ifdef __cplusplus > /* RAII sugar for JS_WriteStructuredClone. */ > +class JS_FRIEND_API(JSAutoStructuredCloneBuffer) { Don't you want this to be JS_PUBLIC_API?
Attachment #544840 -
Flags: review?(bent.mozilla) → review+
Reporter | ||
Comment 3•13 years ago
|
||
(In reply to comment #2) > Don't you want this to be JS_PUBLIC_API? Yeah, what's wrong with me :P
Updated•13 years ago
|
Attachment #544840 -
Flags: review?(jorendorff) → review+
Reporter | ||
Comment 4•13 years ago
|
||
One patch per bug, so I'm going to re-title this and spawn a new bug for the remaining work item. (Will land this patch after new web workers sticks to avoid rebase pain for bent.)
Summary: remove remaining IDB structured-clone deserialization to the main thread → JSAutoStructuredCloneBuffer shouldn't require a 'cx'
Reporter | ||
Comment 5•13 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/2e0fea2cbd9b
Whiteboard: [inbound]
Reporter | ||
Comment 6•13 years ago
|
||
Backed out (b/c of bug 662852) http://hg.mozilla.org/integration/mozilla-inbound/rev/ac34cb0bb085 Relanded with gross JSInt* types http://hg.mozilla.org/integration/mozilla-inbound/rev/eaf738a0e1b3
Comment 7•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/eaf738a0e1b3
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•