WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 49635
Profiler implementation is fragile
https://bugs.webkit.org/show_bug.cgi?id=49635
Summary
Profiler implementation is fragile
Gavin Barraclough
Reported
2010-11-16 17:43:49 PST
The profile presently requires the exception handling mechanism to explicitly remove all stack frames that are exited during the exception unwind mechanism. This is fragile in a number of ways: * We have to change bytecode register allocation when compiling code to run when profiling, to preserve the callee function (this is also required to call did_call after the call has returned). * In the JIT we have to maintain additional data structures (CodeBlock::RareData::m_functionRegisterInfos) to map back to the register containing the callee. * In the interpreter we use 'magic values' to offset into the instruction stream to rediscover the register containing the function. Instead, move profiling into the head and tail of functions. * This correctly accounts the cost of the call itself to the caller. * This allows us to access the callee function object from the callframe. * This means that at the point a call is made we can track the stack depth on the ProfileNode. * When unwinding we can simply report the depth at which the exception is being handled - all call frames above this level are freed.
Attachments
the patch
(49.05 KB, patch)
2010-11-16 17:57 PST
,
Gavin Barraclough
no flags
Details
Formatted Diff
Diff
New improved implementation – breaks no tests, even fixes one!
(37.10 KB, patch)
2010-11-18 15:55 PST
,
Gavin Barraclough
oliver
: review-
Details
Formatted Diff
Diff
Ooops, cleaned up some unnecessary changes.
(33.65 KB, patch)
2010-11-18 15:59 PST
,
Gavin Barraclough
oliver
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Gavin Barraclough
Comment 1
2010-11-16 17:57:18 PST
Created
attachment 74073
[details]
the patch Manually tested on sunspider, test case to check exception handling okay.
Gavin Barraclough
Comment 2
2010-11-16 19:03:52 PST
fixed in
r72160
Ryosuke Niwa
Comment 3
2010-11-16 21:36:27 PST
This change seems to be responsible for a whole bunch of SL test failures:
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r72164%20(20945)/results.html
Could you look into this?
Csaba Osztrogonác
Comment 4
2010-11-17 03:12:05 PST
reopen because it was rolled out by
http://trac.webkit.org/changeset/72176
Csaba Osztrogonác
Comment 5
2010-11-17 03:12:29 PST
Comment on
attachment 74073
[details]
the patch remove r+ from landed and rolled out patch
Gavin Barraclough
Comment 6
2010-11-18 15:55:10 PST
Created
attachment 74308
[details]
New improved implementation – breaks no tests, even fixes one!
Oliver Hunt
Comment 7
2010-11-18 15:57:05 PST
Comment on
attachment 74308
[details]
New improved implementation – breaks no tests, even fixes one! View in context:
https://bugs.webkit.org/attachment.cgi?id=74308&action=review
> JavaScriptCore/interpreter/Interpreter.cpp:566 > +// if (Profiler* profiler = *Profiler::enabledProfilerReference()) { > +// if (callFrame->callee()) > +// profiler->didExecute(callFrame, callFrame->callee()); > +// else > +// profiler->didExecute(callFrame, codeBlock->ownerExecutable()->sourceURL(), codeBlock->ownerExecutable()->lineNo()); > +// }
really?
Gavin Barraclough
Comment 8
2010-11-18 15:59:27 PST
Created
attachment 74310
[details]
Ooops, cleaned up some unnecessary changes.
Gavin Barraclough
Comment 9
2010-11-18 16:54:50 PST
fixed in
r72351
.
Andy Estes
Comment 10
2010-11-19 08:33:57 PST
Looks like 'plugins/refcount-leaks.html' is failing on Leopard due to this change. Filed
https://bugs.webkit.org/show_bug.cgi?id=49809
.
Andy Estes
Comment 11
2010-11-19 08:38:25 PST
fast/profiler/throw-exception-from-eval.html also fails on Leopard. See
https://bugs.webkit.org/show_bug.cgi?id=49801
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug