Last Comment Bug 660930 - InlineGetProp not being inlined on Mac x86-64
: InlineGetProp not being inlined on Mac x86-64
Status: RESOLVED FIXED
[fixed-in-tracemonkey]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Mac OS X
: -- normal (vote)
: ---
Assigned To: general
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-31 12:14 PDT by Justin Lebar (not reading bugmail)
Modified: 2011-06-14 10:58 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (994 bytes, patch)
2011-05-31 12:32 PDT, Justin Lebar (not reading bugmail)
dmandelin: review+
Details | Diff | Splinter Review

Description Justin Lebar (not reading bugmail) 2011-05-31 12:14:35 PDT
It appears that InlineGetProp in js/src/methodjit/StubCalls.cpp is not being inlined on Mac x64 builds.  When I profile part of Dromaeo DOM using Instruments (the XCode 4 equivalent of Shark), InlineGetProp, called by js::mjit::stubs::GetProp, shows up in the profile.

I presume from the name and its usage that we want this function to be inlined?  Should be easy enough to force, at least on gcc.
Comment 1 David Mandelin [:dmandelin] 2011-05-31 12:19:32 PDT
(In reply to comment #0)
> I presume from the name and its usage that we want this function to be
> inlined?  Should be easy enough to force, at least on gcc.

In general it's hard to predict whether inlining a big function will matter. But it's worth a try to see if it helps.
Comment 2 Justin Lebar (not reading bugmail) 2011-05-31 12:31:31 PDT
It seems to help a bit on the Dromaeo DOM microbenchmark element.property, which does:

  var ret;
  var elem = document.getElementById('some-element');
  time this operation in a loop:
    ret = elem.id;

Currently scores: 469 467 473 480 472 471
With inlining:    493 497 491 492 490 336

(Assuming the 336 is an outlier...)
Comment 3 Justin Lebar (not reading bugmail) 2011-05-31 12:32:32 PDT
Created attachment 536374 [details] [diff] [review]
Patch v1
Comment 4 Justin Lebar (not reading bugmail) 2011-05-31 12:38:36 PDT
The units on the numbers from comment 2 are whatever Dromaeo uses -- I think it's runs/s.
Comment 5 Justin Lebar (not reading bugmail) 2011-05-31 12:46:29 PDT
I'm not familiar with the process for landing JS changes.  Do I need to land on TM, or can I land directly on MC?
Comment 6 David Mandelin [:dmandelin] 2011-05-31 13:05:48 PDT
(In reply to comment #5)
> I'm not familiar with the process for landing JS changes.  Do I need to land
> on TM, or can I land directly on MC?

Either is acceptable, but TM is generally preferred for JS changes.
Comment 7 Justin Lebar (not reading bugmail) 2011-05-31 16:58:55 PDT
http://hg.mozilla.org/tracemonkey/rev/a0f2364ea669
Comment 8 Justin Lebar (not reading bugmail) 2011-06-14 10:58:11 PDT
This was pushed in some TM merge; closing.

Note You need to log in before you can comment on or make changes to this bug.