Make it easier to use NoLockingNecessary
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Jun 2016 18:41:16 +0000 (18:41 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Jun 2016 18:41:16 +0000 (18:41 +0000)
commitea66668d4f6497de0b0c0071ae20bcfb150a581f
tree6de3b554cb07e9c3ba5110c14fea3808e011544e
parent2631666c339114edd0adc54723f50c695825ac97
Make it easier to use NoLockingNecessary
https://bugs.webkit.org/show_bug.cgi?id=158306

Reviewed by Keith Miller.

Source/JavaScriptCore:

Adapt to the new NoLockingNecessary API. More details in the WTF ChangeLog.

* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
(JSC::BytecodeGenerator::initializeArrowFunctionContextScopeIfNeeded):
(JSC::BytecodeGenerator::instantiateLexicalVariables):
(JSC::BytecodeGenerator::emitPrefillStackTDZVariables):
(JSC::BytecodeGenerator::initializeBlockScopedFunctions):
(JSC::BytecodeGenerator::hoistSloppyModeFunctionIfNecessary):
(JSC::BytecodeGenerator::popLexicalScopeInternal):
(JSC::BytecodeGenerator::prepareLexicalScopeForNextForLoopIteration):
(JSC::BytecodeGenerator::variable):
(JSC::BytecodeGenerator::createVariable):
(JSC::BytecodeGenerator::emitResolveScope):
(JSC::BytecodeGenerator::emitPushFunctionNameScope):
* runtime/ConcurrentJITLock.h:
(JSC::ConcurrentJITLockerBase::ConcurrentJITLockerBase):
(JSC::ConcurrentJITLocker::ConcurrentJITLocker):

Source/WTF:

An idiom that we borrowed from LLVM is that if a function requires a lock to be held, we
have it take a const Locker& as its first argument. This may not communicate which lock is
to be held, but it does help us to remember that some lock must be held. So far, it's been
relatively easy to then figure out which lock. We've had bugs where we forgot to hold a
lock but I don't remember the last time we had a bug where we grabbed the wrong lock.

But sometimes, we know at the point where we call such a method that we actually don't
need to hold any lock. This usually happens during object construction. If we're
constructing some object then we usually know that we have not escaped it yet, so we don't
need to waste time acquiring its lock. We could solve this by having a separate set of
methods that don't do or require locking. This would be cumbersome, since usually for
every variant that takes const Locker&, there is already one that doesn't, and that one
will grab the lock for you. So this means having a third variant, that also doesn't take a
const Locker&, but does no locking. That's pretty weird.

So, we introduced NoLockingNecessary for situations like this. The idiom went like so:

    Locker<Whatever> locker(Locker<Whatever>::NoLockingNecessary)
    stuff->foo(locker);

Usually though, there would be some distance between where the locker is defined and where
it's used, so when you just look at stuff->foo(locker) in isolation you don't know if this
is a real locker or a NoLockingNecessary cast. Also, requiring two lines for this just
adds code.

This change makes this easier. Now you can just do:

    stuff->foo(NoLockingNecessary).

This is because NoLockingNecessary has been pulled out into the WTF namespace (and is
usinged from the global namespace) and the Locker<> constructor that takes
NoLockingNecessaryTag is now implicit.

The only possible downside of this change is that people might use this idiom more
frequently now that it's easier to use. I don't think that's a bad thing. I'm now
convinced that this is not a bad idiom. When I was fixing an unrelated bug, I almost went
the way of adding more locking to some core JSC data structures, and in the process, I
needed to use NoLockingNecessary. It's clear that this is a general-purpose idiom and we
should not impose artificial constraints on its use.

* wtf/Locker.h:
(WTF::Locker::Locker):
(WTF::Locker::~Locker):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@201605 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
Source/JavaScriptCore/runtime/ConcurrentJITLock.h
Source/WTF/ChangeLog
Source/WTF/wtf/Locker.h