1363956 - Check the flag before calling expensive IsWindowProxy() in CheckedUnwrap()

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P2)

Description

[Ting-Yu Chou [:ting] (away)]

From the profile of test case attachment 8848224 [details], I see IsWindowProxy() under CheckedUnwrap() which is form GenericBindingGetter(), it can be avoid by checking |stopAtWindowProxy| at first.

Comment 1

[Ting-Yu Chou [:ting] (away)]

Attachment: Bug 1363956 - Check the flag before calling relatively expensive IsWindowProxy().

Review commit: https://reviewboard.mozilla.org/r/138220/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/138220/

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8866615 [details]
Bug 1363956 - Check the flag before calling relatively expensive IsWindowProxy().

https://reviewboard.mozilla.org/r/138220/#review141884

::: commit-message-0b255:1
(Diff revision 1)
> +Bug 1363956 - Check the flag before calling expensive IsWindowProxy(). r?bz

How about:

  Check the stopAtWindowProxy flag before checking IsWindowProxy(), since the flag check is cheaper and in performance-sensitive binding code the flag is false.
  
for the commit message?

Comment 3

[Ting-Yu Chou [:ting] (away)]

Comment on attachment 8866615 [details]
Bug 1363956 - Check the flag before calling relatively expensive IsWindowProxy().

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/138220/diff/1-2/

Comment 4

[Ting-Yu Chou [:ting] (away)]

Comment on attachment 8866615 [details]
Bug 1363956 - Check the flag before calling relatively expensive IsWindowProxy().

https://reviewboard.mozilla.org/r/138220/#review141884

> How about:
> 
>   Check the stopAtWindowProxy flag before checking IsWindowProxy(), since the flag check is cheaper and in performance-sensitive binding code the flag is false.
>   
> for the commit message?

Added. I was kind of nervous because of your nick, the patch is straitghtforward which I didn't know how to make a more decent message, I've learned and thanks about that.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e1bb35dd44fc
Check the flag before calling relatively expensive IsWindowProxy(). r=bz

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e1bb35dd44fc