660930 - InlineGetProp not being inlined on Mac x86-64

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Justin Lebar (not reading bugmail)]

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]]

(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)]

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)]

Attachment: Patch v1

Comment 4

[Justin Lebar (not reading bugmail)]

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)]

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]]

(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)]

http://hg.mozilla.org/tracemonkey/rev/a0f2364ea669

Comment 8

[Justin Lebar (not reading bugmail)]

This was pushed in some TM merge; closing.