461563 - Allow WrapNative to return a jsval without the wrapper

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P1)

Description

[Peter Van der Beken [:peterv]]

Attachment: v1

A lot of callers just want a jsval, and have a rooted jsval*. Currently they get the XPCWrappedNative and get the jsval from that, if we can return the jsval without the wrapper we can avoid an AddRef/Release of the cached wrappers.

Comment 1

[Peter Van der Beken [:peterv]]

Attachment: v1.1

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 349021 [details] [diff] [review]
v1.1

- In nsDOMClassInfo::WrapNative():

-  *aHolder = nsnull;
-  
   if (!native) {
     *vp = JSVAL_NULL;
+    if (aHolder) {
+      *aHolder = nsnull;
+    }

I say we just remove this code, all callers of this pass in nsCOMPtr's which are null to begin with.

- In XPCConvert::NativeInterface2JSObject():

+    *d = JSVAL_NULL;
+    if(dest)
+        *dest = nsnull;
     if(!src)
+    {
         return JS_TRUE;
+    }

Surrounding code seems to suggest no brackets in this single line if.

The rest looks good! r+sr=jst

Comment 3

[Peter Van der Beken [:peterv]]

We really want these performance improvements in 1.9.1.

Comment 4

[Johnny Stenback (:jst)]

Yup, blocking.

Comment 5

[Peter Van der Beken [:peterv]]

Attachment: v1.1

Comment 6

[Shawn Wilsher :sdwilsh]

http://hg.mozilla.org/mozilla-central/rev/a4495a0cf2ff

Comment 7

[Shawn Wilsher :sdwilsh]

backed this out while trying to identify a perf regression (bug 467102):
http://hg.mozilla.org/mozilla-central/rev/6dc4b85cd8a7

Comment 8

[Shawn Wilsher :sdwilsh]

See bug 467102 comment 6 for an explanation about the perf regression.

Comment 9

[Peter Van der Beken [:peterv]]

I just tried to reproduce the regression on the try server, but it doesn't show any difference on Twinopen for Windows XP (trunk: 251.44, trunk with patch: 251.11).

Comment 10

[Shawn Wilsher :sdwilsh]

It's likely going to be worthwhile to try and land again when the tree is quiet.

Comment 11

[Peter Van der Beken [:peterv]]

I checked this back in, the Txul numbers look a little bit higher and more unstable on WinXP (not on Vista?). It's hard to tell if it's a real regression or just noise :-/. Maybe I should try to back out the patch in pieces, to see if any particular part makes the numbers more unstable.