Need a store-load fence between setting cell state and visiting the object in SlotVisitor
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 24 Sep 2016 00:47:30 +0000 (00:47 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 24 Sep 2016 00:47:30 +0000 (00:47 +0000)
commit1c216bc6f2195f1189049a251b9c80168bec6d49
treede2b2d91f34dc254f0ef2558e6e66402f403c022
parent4cfea9ce9a649487f69f04813e64888415d981ea
Need a store-load fence between setting cell state and visiting the object in SlotVisitor
https://bugs.webkit.org/show_bug.cgi?id=162354

Reviewed by Mark Lam.

Source/JavaScriptCore:

This was meant to be a small change, but then it became bigger as I found small
opportunities for improving this code. This adds a store-load fence and is performance-
neutral. That's probably partly due to other optimizations that I did to visitChildren().

Initially, I found that adding an mfence as a store-load fence was terribly expensive. So,
I thought that I needed to buffer up a bunch of objects, set their states, do one mfence,
and then visit all of them. This seemed like a win, so I went with it. Unfortunately, this
made no sense for two reasons:

- I shouldn't use mfence. I should use ortop (lock orl $0, (%rsp)) instead. Ortop is
  basically free, and it's what WTF now uses for storeLoadFence().

- My data saying that buffering up objects was not a slow-down was wrong. That was actually
  almost as expensive as the mfence.

But in order to implement that, I made some other improvements that I think we should stick
with:

- SlotVisitor::visitChildren() now uses a switch on type. This replaces what used to be
  some nasty ClassInfo look-ups.

- We no longer save the object's old CellState. We would do that so that we would know what
  state the object had been before we blackened it. But I believe that the more logical
  solution is to have two kinds of black - one for black-for-the-first-time objects and one
  for repeat offenders. This is a lot easier to reason about, since you can now just figure
  this out by looking at the cell directly.

The latter change meant rewiring a bunch of barriers. It didn't make them any more
expensive.

Relanding after fixing a nasty build failure in cloop and elsewhere.

* JavaScriptCore.xcodeproj/project.pbxproj:
* assembler/AbstractMacroAssembler.h:
(JSC::isARMv7IDIVSupported): Deleted.
(JSC::isARM64): Deleted.
(JSC::isX86): Deleted.
(JSC::isX86_64): Deleted.
(JSC::optimizeForARMv7IDIVSupported): Deleted.
(JSC::optimizeForARM64): Deleted.
(JSC::optimizeForX86): Deleted.
(JSC::optimizeForX86_64): Deleted.
* assembler/CPU.h: Added.
(JSC::isARMv7IDIVSupported):
(JSC::isARM64):
(JSC::isX86):
(JSC::isX86_64):
(JSC::optimizeForARMv7IDIVSupported):
(JSC::optimizeForARM64):
(JSC::optimizeForX86):
(JSC::optimizeForX86_64):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::emitStoreBarrier):
* heap/CellState.h:
(JSC::isBlack):
(JSC::blacken):
* heap/Heap.cpp:
(JSC::Heap::addToRememberedSet):
* heap/Heap.h:
* heap/HeapInlines.h:
(JSC::Heap::writeBarrier):
(JSC::Heap::reportExtraMemoryVisited):
(JSC::Heap::reportExternalMemoryVisited):
* heap/MarkStack.cpp:
* heap/MarkStack.h:
* heap/SlotVisitor.cpp:
(JSC::SlotVisitor::visitChildren):
* heap/SlotVisitor.h:
* heap/SlotVisitorInlines.h:
(JSC::SlotVisitor::reportExtraMemoryVisited):
(JSC::SlotVisitor::reportExternalMemoryVisited):
* jit/AssemblyHelpers.h:
(JSC::AssemblyHelpers::jumpIfIsRememberedOrInEden):
* llint/LLIntData.cpp:
(JSC::LLInt::Data::performAssertions):
* llint/LowLevelInterpreter.asm:
* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:
* runtime/JSObject.h:
(JSC::isJSFinalObject):

Source/WTF:

Fix this on x86-32.

* wtf/Atomics.h:
(WTF::x86_ortop):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@206344 268f45cc-cd09-0410-ab3c-d52691b4dbfc
22 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
Source/JavaScriptCore/assembler/AbstractMacroAssembler.h
Source/JavaScriptCore/assembler/CPU.h [new file with mode: 0644]
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/heap/CellState.h
Source/JavaScriptCore/heap/Heap.cpp
Source/JavaScriptCore/heap/Heap.h
Source/JavaScriptCore/heap/HeapInlines.h
Source/JavaScriptCore/heap/MarkStack.cpp
Source/JavaScriptCore/heap/MarkStack.h
Source/JavaScriptCore/heap/SlotVisitor.cpp
Source/JavaScriptCore/heap/SlotVisitor.h
Source/JavaScriptCore/heap/SlotVisitorInlines.h
Source/JavaScriptCore/jit/AssemblyHelpers.h
Source/JavaScriptCore/llint/LLIntData.cpp
Source/JavaScriptCore/llint/LowLevelInterpreter.asm
Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm
Source/JavaScriptCore/llint/LowLevelInterpreter64.asm
Source/JavaScriptCore/runtime/JSObject.h
Source/WTF/ChangeLog
Source/WTF/wtf/Atomics.h