Bug 142674 – Replace TCSpinLock with a new WTF::SpinLock based on WTF::Atomic

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Mark Lam]

We no longer use TCMalloc in our code, and we now have C++11.  This replaces the TCMalloc_SpinLock with a new WTF::SpinLock based on WTF::Atomic (which is a wrapper around std::atomic).

[Mark Lam]

Created attachment 248592 [details]
the patch.

[Mark Lam]

Created attachment 248593 [details]
patch 2: added some ChangeLog comment.

[Filip Pizlo]

Comment on attachment 248593 [details]
patch 2: added some ChangeLog comment.

Can you also change this SpinLock to using methods named according to our conventions (i.e. lock() instead of Lock())?  Then you could get rid of SpinLockHolder and use the existing WTF holder.

[Mark Lam]

(In reply to comment #3)
> Comment on attachment 248593 [details]
> patch 2: added some ChangeLog comment.
> 
> Can you also change this SpinLock to using methods named according to our
> conventions (i.e. lock() instead of Lock())?  Then you could get rid of
> SpinLockHolder and use the existing WTF holder.

I did.  Look at SpinLock.h.  I think you may be looking at the deleted TCSpinLock.h.  SpinLockHolder is now implemented on top of WTF::Locker just like we do with MutexLocker.  I opted not to rename it to SpinLockLocker because this reduces unnecessary diffs.

[Mark Lam]

Thanks for the review.  Landed in r181485: <http://trac.webkit.org/r181485>.

[Chris Dumez]

This may have caused Speedometer to start crashing:
Running Speedometer/Full.html (144 of 144)
error: Speedometer/Full.html
1   0x113d6747e JSC::DFG::(anonymous namespace)::PutStackSinkingPhase::run()
2   0x113d5ded4 JSC::DFG::performPutStackSinking(JSC::DFG::Graph&)
3   0x113db9d99 JSC::DFG::Plan::compileInThreadImpl(JSC::DFG::LongLivedState&)
4   0x113db96dd JSC::DFG::Plan::compileInThread(JSC::DFG::LongLivedState&, JSC::DFG::ThreadData*)
5   0x113e45042 JSC::DFG::Worklist::runThread(JSC::DFG::ThreadData*)
6   0x1141cf833 WTF::threadEntryPoint(void*)
7   0x1141cfd1f WTF::wtfThreadEntryPoint(void*)
8   0x7fff8e583268 _pthread_body
9   0x7fff8e5831e5 _pthread_body
10  0x7fff8e58141d thread_start

It is consistently crashing on all perf bots:
https://build.webkit.org/builders/Apple%20Yosemite%20Release%20WK2%20%28Perf%29/builds/1263/steps/perf-test/logs/stdio

There are a few other commits in the range but this one seems the most likely cause.

[Chris Dumez]

(In reply to comment #6)
> This may have caused Speedometer to start crashing:
> Running Speedometer/Full.html (144 of 144)
> error: Speedometer/Full.html
> 1   0x113d6747e JSC::DFG::(anonymous namespace)::PutStackSinkingPhase::run()
> 2   0x113d5ded4 JSC::DFG::performPutStackSinking(JSC::DFG::Graph&)
> 3   0x113db9d99
> JSC::DFG::Plan::compileInThreadImpl(JSC::DFG::LongLivedState&)
> 4   0x113db96dd JSC::DFG::Plan::compileInThread(JSC::DFG::LongLivedState&,
> JSC::DFG::ThreadData*)
> 5   0x113e45042 JSC::DFG::Worklist::runThread(JSC::DFG::ThreadData*)
> 6   0x1141cf833 WTF::threadEntryPoint(void*)
> 7   0x1141cfd1f WTF::wtfThreadEntryPoint(void*)
> 8   0x7fff8e583268 _pthread_body
> 9   0x7fff8e5831e5 _pthread_body
> 10  0x7fff8e58141d thread_start
> 
> It is consistently crashing on all perf bots:
> https://build.webkit.org/builders/
> Apple%20Yosemite%20Release%20WK2%20%28Perf%29/builds/1263/steps/perf-test/
> logs/stdio
> 
> There are a few other commits in the range but this one seems the most
> likely cause.

Filed https://bugs.webkit.org/show_bug.cgi?id=142697