Remove atomic increment/decrement on each DOM call

Remove atomic increment/decrement on each DOM call
    
    This patch removes the VMState stack object for each callbacks into the
    embedder.  When we call an interceptor callback, we don't need to ratelimit the
    profiler (because the vast majority of interceptor callbacks are quick), and
    therefore we can avoid the atomic increment and decrement operations.
    
    This change removes a function that accounts for 7% of Dromeo's dom-traverse
    profile on Mac OS X.

[abarth-chromium, 2012-11-26 20:38:17]

I've updated this CL with a simpler approach.  As far as I can tell, we don't
need to create these VMState objects at all.

[abarth-chromium, 2012-11-26 20:46:37]

I've run this through make ia32.release.check and all the WebKit LayoutTests,
and everything passed.

[abarth-chromium, 2012-11-29 01:57:53]

I got a fancier profiler for Linux, and we spend 7.7% of our time in these
atomic operations there too.

[abarth-chromium, 2012-11-29 02:26:21]

Here's the combined effect on the profile from this CL and
<https://codereview.chromium.org/11434021>:

== Before ==

     198   7.7%   7.7%      344  13.4% v8::internal::Isolate::SetCurrentVMState
(inline)
     165   6.4%  14.2%      218   8.5% toV8Fast (inline)
     155   6.0%  20.2%      179   7.0%
v8::Object::GetAlignedPointerFromInternalField
     131   5.1%  25.3%      134   5.2%
v8::internal::RuntimeProfiler::IsolateEnteredJS (inline)
      83   3.2%  28.6%      214   8.4% nextSiblingAttrGetter
      80   3.1%  31.7%      201   7.8% previousSiblingAttrGetter
      58   2.3%  34.0%       58   2.3% CreateHandle (inline)
      50   2.0%  35.9%       50   2.0% 0x00002715b6e0e8cf
      47   1.8%  37.7%      701  27.4%
v8::internal::JSObject::GetElementWithInterceptor
      43   1.7%  39.4%      114   4.4% lastChildAttrGetter

== After ==

     216   8.4%   8.4%      263  10.3% toV8Fast (inline)
      94   3.7%  12.1%      120   4.7%
v8::Object::GetAlignedPointerFromInternalField (inline)
      79   3.1%  15.2%       79   3.1% v8::internal::Object::IsOddball (inline)
      63   2.5%  17.6%       63   2.5% CreateHandle (inline)
      56   2.2%  19.8%      101   3.9% v8::internal::Isolate::SetCurrentVMState
(inline)
      55   2.1%  22.0%       55   2.1%
v8::internal::FunctionTemplateInfo::indexed_property_handler (inline)
      54   2.1%  24.1%      122   4.8%
WebCore::LiveNodeListBase::itemBeforeOrAfterCachedItem
      52   2.0%  26.1%      629  24.6%
v8::internal::JSObject::GetElementWithInterceptor
      42   1.6%  27.8%       42   1.6%
v8::internal::RuntimeProfiler::IsolateEnteredJS (inline)
      39   1.5%  29.3%       39   1.5% 0x000017932200e8cf

Notice that in the "before" profile, we're burning a ton of samples in
SetCurrentVMState and IsolateEnteredJS.  In the "after" profile, toV8Fast
accounts for a much larger fraction of the samples.

[Sven Panne, 2012-11-29 08:26:33]

How exactly did you profile this? I'd be interested in trying out some things,
but I don't have a black belt in profiling Chrome as a whole yet... :-)

[abarth-chromium, 2012-11-29 15:56:30]

On 2012/11/29 08:26:33, Sven Panne wrote:
> How exactly did you profile this? I'd be interested in trying out some things,
> but I don't have a black belt in profiling Chrome as a whole yet... :-)

1) Build the Chromium port of WebKit for Linux:
https://trac.webkit.org/wiki/Chromium
2) sudo apt-get install google-perftools
3) ./Tools/Scripts/run-perf-tests Dromeo/dom-traverse.html --profile

The --profile option is very new, so it might still have some rough edges.  If
you want backtraces, you might need to run "update-webkit --chromium" and
"build-webkit --chromium" with GYP_DEFINES="profiling=1" in your environment,
but basic profiling should work without that.

To profile on Mac OS X, you need to export GYP_DEFINES="mac_strip_release=0"
instead of profiling to get symbols in your build.  You can then use Instruments
(which comes with Xcode) to profile DumpRenderTree as you would any other
executable.

[Yang, 2012-11-29 16:43:19]

On 2012/11/29 15:56:30, abarth wrote:
> On 2012/11/29 08:26:33, Sven Panne wrote:
> > How exactly did you profile this? I'd be interested in trying out some
things,
> > but I don't have a black belt in profiling Chrome as a whole yet... :-)
> 
> 1) Build the Chromium port of WebKit for Linux:
> https://trac.webkit.org/wiki/Chromium
> 2) sudo apt-get install google-perftools
> 3) ./Tools/Scripts/run-perf-tests Dromeo/dom-traverse.html --profile
> 
> The --profile option is very new, so it might still have some rough edges.  If
> you want backtraces, you might need to run "update-webkit --chromium" and
> "build-webkit --chromium" with GYP_DEFINES="profiling=1" in your environment,
> but basic profiling should work without that.
> 
> To profile on Mac OS X, you need to export GYP_DEFINES="mac_strip_release=0"
> instead of profiling to get symbols in your build.  You can then use
Instruments
> (which comes with Xcode) to profile DumpRenderTree as you would any other
> executable.

I recently landed a CL that hooks VMState to catch places where we call into
DOM, for profiling purposes. It would be bad if those state changes are removed
entirely. Would it be possible to log state changes only when profiling flags
are set so that for profiling, I can still rely on those state changes, but for
the usual case, we could do away with the atomic increment and decrement?

[abarth-chromium, 2012-11-29 18:32:07]

> I recently landed a CL that hooks VMState to catch places where we call into
> DOM, for profiling purposes. It would be bad if those state changes are
removed
> entirely. Would it be possible to log state changes only when profiling flags
> are set so that for profiling, I can still rely on those state changes, but
for
> the usual case, we could do away with the atomic increment and decrement?

I don't mind your introducing hooks for profiling as long as they don't slow
down DOM operations in production.  Ideally, the profiling hooks would be
compiled out in normal builds, but I'm not sure if that's consistent with how
you've got profiling set up.

These code paths are extremely hot.  We see wins on our benchmarks when we
remove individual branches.  IMHO, we should remove these hooks, get numbers for
the benchmarks we care about, and then make sure we don't regress those
benchmarks when we add back the profiling hooks.

[Sven Panne, 2012-12-11 08:14:48]

Closing this one because of a more general approach currently in the works...
(which probably removes *all* locking/atomic ops in this area)