<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[206314] trunk/Source</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/206314">206314</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2016-09-23 11:09:44 -0700 (Fri, 23 Sep 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>
Source/JavaScriptCore:
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.
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.
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::emitStoreBarrier):
* heap/CellState.h:
(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:
REGRESSION(<a href="http://trac.webkit.org/projects/webkit/changeset/194387">r194387</a>): Crash on github.com in IntlDateTimeFormat::resolvedOptions in C locale
https://bugs.webkit.org/show_bug.cgi?id=162139
Patch by Carlos Garcia Campos <cgarcia@igalia.com> on 2016-09-23
Reviewed by Michael Catanzaro.
Handle the case of "C" or "POSIX" locale and use "en-US" as default. That matches what ICU and other ports do,
as well as what layout tests expect (some tests like js/intl-collator.html pass in the bots only because we use
en-US as system locale in those bots).
* wtf/PlatformUserPreferredLanguagesUnix.cpp:
(WTF::platformLanguage):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLLowerDFGToB3cpp">trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapCellStateh">trunk/Source/JavaScriptCore/heap/CellState.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeapcpp">trunk/Source/JavaScriptCore/heap/Heap.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeaph">trunk/Source/JavaScriptCore/heap/Heap.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeapInlinesh">trunk/Source/JavaScriptCore/heap/HeapInlines.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMarkStackcpp">trunk/Source/JavaScriptCore/heap/MarkStack.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMarkStackh">trunk/Source/JavaScriptCore/heap/MarkStack.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapSlotVisitorcpp">trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapSlotVisitorh">trunk/Source/JavaScriptCore/heap/SlotVisitor.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapSlotVisitorInlinesh">trunk/Source/JavaScriptCore/heap/SlotVisitorInlines.h</a></li>
<li><a href="#trunkSourceJavaScriptCorejitAssemblyHelpersh">trunk/Source/JavaScriptCore/jit/AssemblyHelpers.h</a></li>
<li><a href="#trunkSourceJavaScriptCorellintLLIntDatacpp">trunk/Source/JavaScriptCore/llint/LLIntData.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorellintLowLevelInterpreterasm">trunk/Source/JavaScriptCore/llint/LowLevelInterpreter.asm</a></li>
<li><a href="#trunkSourceJavaScriptCorellintLowLevelInterpreter32_64asm">trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm</a></li>
<li><a href="#trunkSourceJavaScriptCorellintLowLevelInterpreter64asm">trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSObjecth">trunk/Source/JavaScriptCore/runtime/JSObject.h</a></li>
<li><a href="#trunkSourceWTFChangeLog">trunk/Source/WTF/ChangeLog</a></li>
<li><a href="#trunkSourceWTFwtfAtomicsh">trunk/Source/WTF/wtf/Atomics.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -1,3 +1,69 @@
</span><ins>+2016-09-22 Filip Pizlo <fpizlo@apple.com>
+
+ 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.
+
+ 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.
+
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::emitStoreBarrier):
+ * heap/CellState.h:
+ (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):
+
</ins><span class="cx"> 2016-09-23 Csaba Osztrogonác <ossy@webkit.org>
</span><span class="cx">
</span><span class="cx"> ARM EABI buildfix after r206289
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLLowerDFGToB3cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -11435,7 +11435,8 @@
</span><span class="cx"> LBasicBlock continuation = m_out.newBlock();
</span><span class="cx">
</span><span class="cx"> m_out.branch(
</span><del>- m_out.notZero32(loadCellState(base)), usually(continuation), rarely(slowPath));
</del><ins>+ m_out.above(loadCellState(base), m_out.constInt32(blackThreshold)),
+ usually(continuation), rarely(slowPath));
</ins><span class="cx">
</span><span class="cx"> LBasicBlock lastNext = m_out.appendTo(slowPath, continuation);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapCellStateh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/CellState.h (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/CellState.h 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/heap/CellState.h 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2015 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="cx"> * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -26,34 +26,47 @@
</span><span class="cx"> #ifndef CellState_h
</span><span class="cx"> #define CellState_h
</span><span class="cx">
</span><ins>+#include <wtf/Assertions.h>
+
</ins><span class="cx"> namespace JSC {
</span><span class="cx">
</span><span class="cx"> enum class CellState : uint8_t {
</span><del>- // The object is black as far as this GC is concerned. When not in GC, this just means that it's an
- // old gen object. Note that we deliberately arrange OldBlack to be zero, so that the store barrier on
- // a target object "from" is just:
- //
- // if (!from->cellState())
- // slowPath(from);
- //
- // There is a bunch of code in the LLInt and JITs that rely on this being the case. You'd have to
- // change a lot of code if you ever wanted the store barrier to be anything but a non-zero check on
- // cellState.
- OldBlack = 0,
</del><ins>+ // The object is black for the first time during this GC.
+ NewBlack = 0,
</ins><span class="cx">
</span><ins>+ // The object is black for the Nth time during this full GC cycle (N > 1). An object may get to
+ // this state if it transitions from black back to grey during a concurrent GC, or because it
+ // wound up in the remembered set because of a generational barrier.
+ OldBlack = 1,
+
</ins><span class="cx"> // The object is in eden. During GC, this means that the object has not been marked yet.
</span><del>- NewWhite = 1,
</del><ins>+ NewWhite = 2,
</ins><span class="cx">
</span><ins>+ // The object is grey - i.e. it will be scanned - and this is the first time in this GC that we are
+ // going to scan it. If this is an eden GC, this also means that the object is in eden.
+ NewGrey = 3,
+
</ins><span class="cx"> // The object is grey - i.e. it will be scanned - but it either belongs to old gen (if this is eden
</span><span class="cx"> // GC) or it is grey a second time in this current GC (because a concurrent store barrier requested
</span><span class="cx"> // re-greying).
</span><del>- OldGrey = 2,
-
- // The object is grey - i.e. it will be scanned - and this is the first time in this GC that we are
- // going to scan it. If this is an eden GC, this also means that the object is in eden.
- NewGrey = 3
</del><ins>+ OldGrey = 4
</ins><span class="cx"> };
</span><span class="cx">
</span><ins>+static const unsigned blackThreshold = 1; // x <= blackThreshold means x is black.
+
+inline bool isBlack(CellState cellState)
+{
+ return static_cast<unsigned>(cellState) <= blackThreshold;
+}
+
+inline CellState blacken(CellState cellState)
+{
+ if (cellState == CellState::NewGrey)
+ return CellState::NewBlack;
+ ASSERT(cellState == CellState::NewBlack || cellState == CellState::OldBlack || cellState == CellState::OldGrey);
+ return CellState::OldBlack;
+}
+
</ins><span class="cx"> } // namespace JSC
</span><span class="cx">
</span><span class="cx"> #endif // CellState_h
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.cpp (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.cpp 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/heap/Heap.cpp 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -28,6 +28,7 @@
</span><span class="cx"> #include "FullGCActivityCallback.h"
</span><span class="cx"> #include "GCActivityCallback.h"
</span><span class="cx"> #include "GCIncomingRefCountedSetInlines.h"
</span><ins>+#include "GCSegmentedArrayInlines.h"
</ins><span class="cx"> #include "GCTypeMap.h"
</span><span class="cx"> #include "HasOwnPropertyCache.h"
</span><span class="cx"> #include "HeapHelperPool.h"
</span><span class="lines">@@ -914,7 +915,7 @@
</span><span class="cx"> {
</span><span class="cx"> ASSERT(cell);
</span><span class="cx"> ASSERT(!Options::useConcurrentJIT() || !isCompilationThread());
</span><del>- ASSERT(cell->cellState() == CellState::OldBlack);
</del><ins>+ ASSERT(isBlack(cell->cellState()));
</ins><span class="cx"> // Indicate that this object is grey and that it's one of the following:
</span><span class="cx"> // - A re-greyed object during a concurrent collection.
</span><span class="cx"> // - An old remembered object.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeaph"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.h (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.h 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/heap/Heap.h 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -175,11 +175,11 @@
</span><span class="cx"> // call both of these functions: Calling only one may trigger catastropic
</span><span class="cx"> // memory growth.
</span><span class="cx"> void reportExtraMemoryAllocated(size_t);
</span><del>- void reportExtraMemoryVisited(CellState cellStateBeforeVisiting, size_t);
</del><ins>+ void reportExtraMemoryVisited(JSCell*, size_t);
</ins><span class="cx">
</span><span class="cx"> #if ENABLE(RESOURCE_USAGE)
</span><span class="cx"> // Use this API to report the subset of extra memory that lives outside this process.
</span><del>- void reportExternalMemoryVisited(CellState cellStateBeforeVisiting, size_t);
</del><ins>+ void reportExternalMemoryVisited(JSCell*, size_t);
</ins><span class="cx"> size_t externalMemorySize() { return m_externalMemorySize; }
</span><span class="cx"> #endif
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/HeapInlines.h (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/HeapInlines.h 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/heap/HeapInlines.h 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -125,7 +125,7 @@
</span><span class="cx"> #if ENABLE(WRITE_BARRIER_PROFILING)
</span><span class="cx"> WriteBarrierCounters::countWriteBarrier();
</span><span class="cx"> #endif
</span><del>- if (!from || from->cellState() != CellState::OldBlack)
</del><ins>+ if (!from || !isBlack(from->cellState()))
</ins><span class="cx"> return;
</span><span class="cx"> if (!to || to->cellState() != CellState::NewWhite)
</span><span class="cx"> return;
</span><span class="lines">@@ -135,7 +135,7 @@
</span><span class="cx"> inline void Heap::writeBarrier(const JSCell* from)
</span><span class="cx"> {
</span><span class="cx"> ASSERT_GC_OBJECT_LOOKS_VALID(const_cast<JSCell*>(from));
</span><del>- if (!from || from->cellState() != CellState::OldBlack)
</del><ins>+ if (!from || !isBlack(from->cellState()))
</ins><span class="cx"> return;
</span><span class="cx"> addToRememberedSet(from);
</span><span class="cx"> }
</span><span class="lines">@@ -146,10 +146,10 @@
</span><span class="cx"> reportExtraMemoryAllocatedSlowCase(size);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-inline void Heap::reportExtraMemoryVisited(CellState dataBeforeVisiting, size_t size)
</del><ins>+inline void Heap::reportExtraMemoryVisited(JSCell* cell, size_t size)
</ins><span class="cx"> {
</span><span class="cx"> // We don't want to double-count the extra memory that was reported in previous collections.
</span><del>- if (operationInProgress() == EdenCollection && dataBeforeVisiting == CellState::OldGrey)
</del><ins>+ if (operationInProgress() == EdenCollection && cell->cellState() == CellState::OldBlack)
</ins><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> size_t* counter = &m_extraMemorySize;
</span><span class="lines">@@ -162,10 +162,10 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> #if ENABLE(RESOURCE_USAGE)
</span><del>-inline void Heap::reportExternalMemoryVisited(CellState dataBeforeVisiting, size_t size)
</del><ins>+inline void Heap::reportExternalMemoryVisited(JSCell* cell, size_t size)
</ins><span class="cx"> {
</span><span class="cx"> // We don't want to double-count the external memory that was reported in previous collections.
</span><del>- if (operationInProgress() == EdenCollection && dataBeforeVisiting == CellState::OldGrey)
</del><ins>+ if (operationInProgress() == EdenCollection && cell->cellState() == CellState::OldBlack)
</ins><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> size_t* counter = &m_externalMemorySize;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMarkStackcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MarkStack.cpp (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MarkStack.cpp 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/heap/MarkStack.cpp 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -26,6 +26,7 @@
</span><span class="cx"> #include "config.h"
</span><span class="cx"> #include "MarkStack.h"
</span><span class="cx">
</span><ins>+#include "GCSegmentedArrayInlines.h"
</ins><span class="cx"> #include "JSCInlines.h"
</span><span class="cx">
</span><span class="cx"> namespace JSC {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMarkStackh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MarkStack.h (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MarkStack.h 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/heap/MarkStack.h 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -26,7 +26,7 @@
</span><span class="cx"> #ifndef MarkStack_h
</span><span class="cx"> #define MarkStack_h
</span><span class="cx">
</span><del>-#include "GCSegmentedArrayInlines.h"
</del><ins>+#include "GCSegmentedArray.h"
</ins><span class="cx">
</span><span class="cx"> namespace JSC {
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapSlotVisitorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/heap/SlotVisitor.cpp 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -25,9 +25,9 @@
</span><span class="cx">
</span><span class="cx"> #include "config.h"
</span><span class="cx"> #include "SlotVisitor.h"
</span><del>-#include "SlotVisitorInlines.h"
</del><span class="cx">
</span><span class="cx"> #include "ConservativeRoots.h"
</span><ins>+#include "GCSegmentedArrayInlines.h"
</ins><span class="cx"> #include "HeapCellInlines.h"
</span><span class="cx"> #include "HeapProfiler.h"
</span><span class="cx"> #include "HeapSnapshotBuilder.h"
</span><span class="lines">@@ -36,6 +36,7 @@
</span><span class="cx"> #include "JSObject.h"
</span><span class="cx"> #include "JSString.h"
</span><span class="cx"> #include "JSCInlines.h"
</span><ins>+#include "SlotVisitorInlines.h"
</ins><span class="cx"> #include "SuperSampler.h"
</span><span class="cx"> #include "VM.h"
</span><span class="cx"> #include <wtf/Lock.h>
</span><span class="lines">@@ -296,25 +297,32 @@
</span><span class="cx">
</span><span class="cx"> SetCurrentCellScope currentCellScope(*this, cell);
</span><span class="cx">
</span><del>- m_currentObjectCellStateBeforeVisiting = cell->cellState();
- cell->setCellState(CellState::OldBlack);
</del><ins>+ cell->setCellState(blacken(cell->cellState()));
</ins><span class="cx">
</span><del>- if (isJSString(cell)) {
</del><ins>+ // FIXME: Make this work on ARM also.
+ // https://bugs.webkit.org/show_bug.cgi?id=162461
+ if (isX86())
+ WTF::storeLoadFence();
+
+ switch (cell->type()) {
+ case StringType:
</ins><span class="cx"> JSString::visitChildren(const_cast<JSCell*>(cell), *this);
</span><del>- return;
- }
-
- if (isJSFinalObject(cell)) {
</del><ins>+ break;
+
+ case FinalObjectType:
</ins><span class="cx"> JSFinalObject::visitChildren(const_cast<JSCell*>(cell), *this);
</span><del>- return;
- }
</del><ins>+ break;
</ins><span class="cx">
</span><del>- if (isJSArray(cell)) {
</del><ins>+ case ArrayType:
</ins><span class="cx"> JSArray::visitChildren(const_cast<JSCell*>(cell), *this);
</span><del>- return;
</del><ins>+ break;
+
+ default:
+ // FIXME: This could be so much better.
+ // https://bugs.webkit.org/show_bug.cgi?id=162462
+ cell->methodTable()->visitChildren(const_cast<JSCell*>(cell), *this);
+ break;
</ins><span class="cx"> }
</span><del>-
- cell->methodTable()->visitChildren(const_cast<JSCell*>(cell), *this);
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void SlotVisitor::donateKnownParallel()
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapSlotVisitorh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/SlotVisitor.h (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/SlotVisitor.h 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/heap/SlotVisitor.h 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -168,8 +168,6 @@
</span><span class="cx"> HeapSnapshotBuilder* m_heapSnapshotBuilder { nullptr };
</span><span class="cx"> JSCell* m_currentCell { nullptr };
</span><span class="cx">
</span><del>- CellState m_currentObjectCellStateBeforeVisiting { CellState::NewWhite };
-
</del><span class="cx"> public:
</span><span class="cx"> #if !ASSERT_DISABLED
</span><span class="cx"> bool m_isCheckingForDefaultMarkViolation;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapSlotVisitorInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/SlotVisitorInlines.h (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/SlotVisitorInlines.h 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/heap/SlotVisitorInlines.h 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -106,13 +106,13 @@
</span><span class="cx">
</span><span class="cx"> inline void SlotVisitor::reportExtraMemoryVisited(size_t size)
</span><span class="cx"> {
</span><del>- heap()->reportExtraMemoryVisited(m_currentObjectCellStateBeforeVisiting, size);
</del><ins>+ heap()->reportExtraMemoryVisited(m_currentCell, size);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> #if ENABLE(RESOURCE_USAGE)
</span><span class="cx"> inline void SlotVisitor::reportExternalMemoryVisited(size_t size)
</span><span class="cx"> {
</span><del>- heap()->reportExternalMemoryVisited(m_currentObjectCellStateBeforeVisiting, size);
</del><ins>+ heap()->reportExternalMemoryVisited(m_currentCell, size);
</ins><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitAssemblyHelpersh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/AssemblyHelpers.h (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/AssemblyHelpers.h 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/jit/AssemblyHelpers.h 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -1308,13 +1308,13 @@
</span><span class="cx">
</span><span class="cx"> Jump jumpIfIsRememberedOrInEden(GPRReg cell)
</span><span class="cx"> {
</span><del>- return branchTest8(MacroAssembler::NonZero, MacroAssembler::Address(cell, JSCell::cellStateOffset()));
</del><ins>+ return branch8(Above, Address(cell, JSCell::cellStateOffset()), TrustedImm32(blackThreshold));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> Jump jumpIfIsRememberedOrInEden(JSCell* cell)
</span><span class="cx"> {
</span><span class="cx"> uint8_t* address = reinterpret_cast<uint8_t*>(cell) + JSCell::cellStateOffset();
</span><del>- return branchTest8(MacroAssembler::NonZero, MacroAssembler::AbsoluteAddress(address));
</del><ins>+ return branch8(Above, AbsoluteAddress(address), TrustedImm32(blackThreshold));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // Emits the branch structure for typeof. The code emitted by this doesn't fall through. The
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorellintLLIntDatacpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/llint/LLIntData.cpp (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/llint/LLIntData.cpp 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/llint/LLIntData.cpp 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -214,6 +214,7 @@
</span><span class="cx"> STATIC_ASSERT(GetPutInfo::initializationBits == 0xffc00);
</span><span class="cx">
</span><span class="cx"> STATIC_ASSERT(MarkedBlock::blockSize == 16 * 1024);
</span><ins>+ STATIC_ASSERT(blackThreshold == 1);
</ins><span class="cx">
</span><span class="cx"> ASSERT(bitwise_cast<uintptr_t>(ShadowChicken::Packet::tailMarker()) == static_cast<uintptr_t>(0x7a11));
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorellintLowLevelInterpreterasm"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/llint/LowLevelInterpreter.asm (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/llint/LowLevelInterpreter.asm 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/llint/LowLevelInterpreter.asm 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -409,6 +409,8 @@
</span><span class="cx"> const MarkedBlockSize = 16 * 1024
</span><span class="cx"> const MarkedBlockMask = ~(MarkedBlockSize - 1)
</span><span class="cx">
</span><ins>+const BlackThreshold = 1
+
</ins><span class="cx"> # Allocation constants
</span><span class="cx"> if JSVALUE64
</span><span class="cx"> const JSFinalObjectSizeClassIndex = 1
</span><span class="lines">@@ -888,9 +890,10 @@
</span><span class="cx"> loadb JSCell::m_indexingType[cell], indexingType
</span><span class="cx"> end
</span><span class="cx">
</span><del>-macro skipIfIsRememberedOrInEden(cell, scratch1, scratch2, continuation)
- loadb JSCell::m_cellState[cell], scratch1
- continuation(scratch1)
</del><ins>+macro skipIfIsRememberedOrInEden(cell, slowPath)
+ bba JSCell::m_cellState[cell], BlackThreshold, .done
+ slowPath()
+.done:
</ins><span class="cx"> end
</span><span class="cx">
</span><span class="cx"> macro notifyWrite(set, slow)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorellintLowLevelInterpreter32_64asm"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -500,9 +500,9 @@
</span><span class="cx"> macro writeBarrierOnOperand(cellOperand)
</span><span class="cx"> loadisFromInstruction(cellOperand, t1)
</span><span class="cx"> loadConstantOrVariablePayload(t1, CellTag, t2, .writeBarrierDone)
</span><del>- skipIfIsRememberedOrInEden(t2, t1, t3,
- macro(cellState)
- btbnz cellState, .writeBarrierDone
</del><ins>+ skipIfIsRememberedOrInEden(
+ t2,
+ macro()
</ins><span class="cx"> push cfr, PC
</span><span class="cx"> # We make two extra slots because cCall2 will poke.
</span><span class="cx"> subp 8, sp
</span><span class="lines">@@ -511,8 +511,7 @@
</span><span class="cx"> cCall2Void(_llint_write_barrier_slow)
</span><span class="cx"> addp 8, sp
</span><span class="cx"> pop PC, cfr
</span><del>- end
- )
</del><ins>+ end)
</ins><span class="cx"> .writeBarrierDone:
</span><span class="cx"> end
</span><span class="cx">
</span><span class="lines">@@ -532,9 +531,9 @@
</span><span class="cx">
</span><span class="cx"> loadHelper(t3)
</span><span class="cx">
</span><del>- skipIfIsRememberedOrInEden(t3, t1, t2,
- macro(gcData)
- btbnz gcData, .writeBarrierDone
</del><ins>+ skipIfIsRememberedOrInEden(
+ t3,
+ macro()
</ins><span class="cx"> push cfr, PC
</span><span class="cx"> # We make two extra slots because cCall2 will poke.
</span><span class="cx"> subp 8, sp
</span><span class="lines">@@ -543,8 +542,7 @@
</span><span class="cx"> cCall2Void(_llint_write_barrier_slow)
</span><span class="cx"> addp 8, sp
</span><span class="cx"> pop PC, cfr
</span><del>- end
- )
</del><ins>+ end)
</ins><span class="cx"> .writeBarrierDone:
</span><span class="cx"> end
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorellintLowLevelInterpreter64asm"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -404,16 +404,15 @@
</span><span class="cx"> macro writeBarrierOnOperand(cellOperand)
</span><span class="cx"> loadisFromInstruction(cellOperand, t1)
</span><span class="cx"> loadConstantOrVariableCell(t1, t2, .writeBarrierDone)
</span><del>- skipIfIsRememberedOrInEden(t2, t1, t3,
- macro(cellState)
- btbnz cellState, .writeBarrierDone
</del><ins>+ skipIfIsRememberedOrInEden(
+ t2,
+ macro()
</ins><span class="cx"> push PB, PC
</span><span class="cx"> move t2, a1 # t2 can be a0 (not on 64 bits, but better safe than sorry)
</span><span class="cx"> move cfr, a0
</span><span class="cx"> cCall2Void(_llint_write_barrier_slow)
</span><span class="cx"> pop PC, PB
</span><del>- end
- )
</del><ins>+ end)
</ins><span class="cx"> .writeBarrierDone:
</span><span class="cx"> end
</span><span class="cx">
</span><span class="lines">@@ -432,9 +431,9 @@
</span><span class="cx"> btpz t0, .writeBarrierDone
</span><span class="cx">
</span><span class="cx"> loadHelper(t3)
</span><del>- skipIfIsRememberedOrInEden(t3, t1, t2,
- macro(gcData)
- btbnz gcData, .writeBarrierDone
</del><ins>+ skipIfIsRememberedOrInEden(
+ t3,
+ macro()
</ins><span class="cx"> push PB, PC
</span><span class="cx"> move cfr, a0
</span><span class="cx"> move t3, a1
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSObjecth"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSObject.h (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSObject.h 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/JavaScriptCore/runtime/JSObject.h 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -1093,7 +1093,7 @@
</span><span class="cx">
</span><span class="cx"> inline bool isJSFinalObject(JSCell* cell)
</span><span class="cx"> {
</span><del>- return cell->classInfo() == JSFinalObject::info();
</del><ins>+ return cell->type() == FinalObjectType;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> inline bool isJSFinalObject(JSValue value)
</span></span></pre></div>
<a id="trunkSourceWTFChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/ChangeLog (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WTF/ChangeLog 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/WTF/ChangeLog 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -14,6 +14,18 @@
</span><span class="cx">
</span><span class="cx"> 2016-09-22 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><ins>+ 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.
+
+ Fix this on x86-32.
+
+ * wtf/Atomics.h:
+ (WTF::x86_ortop):
+
+2016-09-22 Filip Pizlo <fpizlo@apple.com>
+
</ins><span class="cx"> Fences on x86 should be a lot cheaper
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=162417
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWTFwtfAtomicsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/wtf/Atomics.h (206313 => 206314)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WTF/wtf/Atomics.h 2016-09-23 17:49:15 UTC (rev 206313)
+++ trunk/Source/WTF/wtf/Atomics.h 2016-09-23 18:09:44 UTC (rev 206314)
</span><span class="lines">@@ -175,10 +175,12 @@
</span><span class="cx"> // know that it is equivalent for our purposes, but it would be good to
</span><span class="cx"> // investigate if that is actually better.
</span><span class="cx"> MemoryBarrier();
</span><del>-#else
</del><ins>+#elif CPU(X86_64)
</ins><span class="cx"> // This has acqrel semantics and is much cheaper than mfence. For exampe, in the JSC GC, using
</span><span class="cx"> // mfence as a store-load fence was a 9% slow-down on Octane/splay while using this was neutral.
</span><span class="cx"> asm volatile("lock; orl $0, (%%rsp)" ::: "memory");
</span><ins>+#else
+ asm volatile("lock; orl $0, (%%esp)" ::: "memory");
</ins><span class="cx"> #endif
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>