=== Auto-CCing suspected CL author yukishiino@chromium.org ===
Hi yukishiino@chromium.org, the bisect results pointed to your CL, please take a look at the
results.
=== BISECT JOB RESULTS ===
Perf regression found with culprit
Suspected Commit
Author : Yuki Shiino
Commit : 25decc66ae3630d275d2757d7c80f052da7b7d9f
Date : Thu Aug 24 12:04:19 2017
Subject: Set the current context to the function's context when entering to LAP.
Bisect Details
Configuration: android_nexus7_perf_bisect
Benchmark : blink_perf.bindings
Metric : undefined-first-child/undefined-first-child
Change : 15.60% | 134.877290584 -> 113.839358733
Revision Result N
chromium@497059 134.877 +- 9.11996 6 good
chromium@497095 129.734 +- 11.0119 6 good
chromium@497113 133.493 +- 10.8869 6 good
chromium@497122 133.476 +- 11.5519 6 good
chromium@497124 130.151 +- 27.2248 9 good
chromium@497124,v8@25decc66ae 109.545 +- 12.9245 6 bad <--
chromium@497124,v8@a653d26984 115.159 +- 9.8585 9 bad
chromium@497124,v8@d8f4e1e1c9 116.488 +- 8.15019 9 bad
chromium@497125 113.057 +- 14.0298 9 bad
chromium@497126 110.94 +- 7.98982 6 bad
chromium@497130 115.566 +- 5.52588 6 bad
chromium@497200 113.839 +- 10.0078 6 bad
To Run This Test
src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests blink_perf.bindings
More information on addressing performance regressions:
http://g.co/ChromePerformanceRegressions
Debug information about this bisect:
https://chromeperf.appspot.com/buildbucket_job_status/8969997871455809056
For feedback, file a bug with component Speed>Bisection
+cc: cbruni@, mstarzinger@,
Hi Camillo and Michael, could you give us advice?
We're now thinking how we can recover the perf regression. Our idea is that, if the lazy accessor's context is the same as the caller's context, we don't need to change the current context at all. As the most cases are the same context, we can skip changing the context.
example)
function F() {
// current context = F.CreationContext()
return obj.lazy_accessor;
}
In this case, if |F| and |obj| have the same creation context, we don't need to change the current context. However, the current implementation (my patch) always looks up |obj|'s constructor and retrieves the creation context of the constructor. Is it possible to detect if we really need to change the current context or not?
https://cs.chromium.org/chromium/src/v8/src/x64/code-stubs-x64.cc?q=is_lazy+file:code-stubs-x64.cc&sq=package:chromium&l=1738
We're wondering if we can replace the code in code-stubs-x64.cc
if (this->is_lazy()) {
with the following way.
if (this->is_lazy() && !this->is_same_context()) {
Is this a feasible idea in the first place?
Sorry for not beeing updating this issue. I had an offline chat with Toon about this issue, and Toon showed an idea to move the context extraction code from assembly to C++ layer, then we should be able to use IC and the regression should be smaller than now and will be acceptable (technically the regression will never be zero, though).
This is on my task queue now.
Comment 1 by 42576172...@developer.gserviceaccount.com
, Aug 28 2017