<!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>[199845] trunk/Source/JavaScriptCore</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/199845">199845</a></dd>
<dt>Author</dt> <dd>sbarati@apple.com</dd>
<dt>Date</dt> <dd>2016-04-21 16:30:36 -0700 (Thu, 21 Apr 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Remove some unnecessary RefPtrs in the parser
https://bugs.webkit.org/show_bug.cgi?id=156865
Reviewed by Filip Pizlo.
The IdentifierArena or the SourceProviderCacheItem will own these UniquedStringImpls
while we are using them. There is no need for us to reference count them.
This might be a 0.5% speedup on octane code-load.
* parser/Parser.cpp:
(JSC::Parser<LexerType>::parseInner):
* parser/Parser.h:
(JSC::Scope::setIsLexicalScope):
(JSC::Scope::isLexicalScope):
(JSC::Scope::closedVariableCandidates):
(JSC::Scope::declaredVariables):
(JSC::Scope::lexicalVariables):
(JSC::Scope::finalizeLexicalEnvironment):
(JSC::Scope::computeLexicallyCapturedVariablesAndPurgeCandidates):
(JSC::Scope::collectFreeVariables):
(JSC::Scope::getCapturedVars):
(JSC::Scope::setStrictMode):
(JSC::Scope::isValidStrictMode):
(JSC::Scope::shadowsArguments):
(JSC::Scope::copyCapturedVariablesToVector):
* parser/SourceProviderCacheItem.h:
(JSC::SourceProviderCacheItem::usedVariables):
(JSC::SourceProviderCacheItem::~SourceProviderCacheItem):
(JSC::SourceProviderCacheItem::create):
(JSC::SourceProviderCacheItem::SourceProviderCacheItem):
(JSC::SourceProviderCacheItem::writtenVariables): Deleted.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreparserParsercpp">trunk/Source/JavaScriptCore/parser/Parser.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreparserParserh">trunk/Source/JavaScriptCore/parser/Parser.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreparserSourceProviderCacheItemh">trunk/Source/JavaScriptCore/parser/SourceProviderCacheItem.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (199844 => 199845)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-04-21 23:28:48 UTC (rev 199844)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-04-21 23:30:36 UTC (rev 199845)
</span><span class="lines">@@ -1,3 +1,38 @@
</span><ins>+2016-04-21 Saam barati <sbarati@apple.com>
+
+ Remove some unnecessary RefPtrs in the parser
+ https://bugs.webkit.org/show_bug.cgi?id=156865
+
+ Reviewed by Filip Pizlo.
+
+ The IdentifierArena or the SourceProviderCacheItem will own these UniquedStringImpls
+ while we are using them. There is no need for us to reference count them.
+
+ This might be a 0.5% speedup on octane code-load.
+
+ * parser/Parser.cpp:
+ (JSC::Parser<LexerType>::parseInner):
+ * parser/Parser.h:
+ (JSC::Scope::setIsLexicalScope):
+ (JSC::Scope::isLexicalScope):
+ (JSC::Scope::closedVariableCandidates):
+ (JSC::Scope::declaredVariables):
+ (JSC::Scope::lexicalVariables):
+ (JSC::Scope::finalizeLexicalEnvironment):
+ (JSC::Scope::computeLexicallyCapturedVariablesAndPurgeCandidates):
+ (JSC::Scope::collectFreeVariables):
+ (JSC::Scope::getCapturedVars):
+ (JSC::Scope::setStrictMode):
+ (JSC::Scope::isValidStrictMode):
+ (JSC::Scope::shadowsArguments):
+ (JSC::Scope::copyCapturedVariablesToVector):
+ * parser/SourceProviderCacheItem.h:
+ (JSC::SourceProviderCacheItem::usedVariables):
+ (JSC::SourceProviderCacheItem::~SourceProviderCacheItem):
+ (JSC::SourceProviderCacheItem::create):
+ (JSC::SourceProviderCacheItem::SourceProviderCacheItem):
+ (JSC::SourceProviderCacheItem::writtenVariables): Deleted.
+
</ins><span class="cx"> 2016-04-21 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> PolymorphicAccess adds sizeof(CallerFrameAndPC) rather than subtracting it when calculating stack height
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreparserParsercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/parser/Parser.cpp (199844 => 199845)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/parser/Parser.cpp 2016-04-21 23:28:48 UTC (rev 199844)
+++ trunk/Source/JavaScriptCore/parser/Parser.cpp 2016-04-21 23:30:36 UTC (rev 199845)
</span><span class="lines">@@ -318,10 +318,10 @@
</span><span class="cx"> #ifndef NDEBUG
</span><span class="cx"> if (m_parsingBuiltin && isProgramParseMode(parseMode)) {
</span><span class="cx"> VariableEnvironment& lexicalVariables = scope->lexicalVariables();
</span><del>- const IdentifierSet& closedVariableCandidates = scope->closedVariableCandidates();
</del><ins>+ const HashSet<UniquedStringImpl*>& closedVariableCandidates = scope->closedVariableCandidates();
</ins><span class="cx"> const BuiltinNames& builtinNames = m_vm->propertyNames->builtinNames();
</span><del>- for (const RefPtr<UniquedStringImpl>& candidate : closedVariableCandidates) {
- if (!lexicalVariables.contains(candidate) && !varDeclarations.contains(candidate) && !builtinNames.isPrivateName(*candidate.get())) {
</del><ins>+ for (UniquedStringImpl* candidate : closedVariableCandidates) {
+ if (!lexicalVariables.contains(candidate) && !varDeclarations.contains(candidate) && !builtinNames.isPrivateName(*candidate)) {
</ins><span class="cx"> dataLog("Bad global capture in builtin: '", candidate, "'\n");
</span><span class="cx"> dataLog(m_source->view());
</span><span class="cx"> CRASH();
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreparserParserh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/parser/Parser.h (199844 => 199845)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/parser/Parser.h 2016-04-21 23:28:48 UTC (rev 199844)
+++ trunk/Source/JavaScriptCore/parser/Parser.h 2016-04-21 23:30:36 UTC (rev 199845)
</span><span class="lines">@@ -297,7 +297,7 @@
</span><span class="cx"> }
</span><span class="cx"> bool isLexicalScope() { return m_isLexicalScope; }
</span><span class="cx">
</span><del>- const IdentifierSet& closedVariableCandidates() const { return m_closedVariableCandidates; }
</del><ins>+ const HashSet<UniquedStringImpl*>& closedVariableCandidates() const { return m_closedVariableCandidates; }
</ins><span class="cx"> VariableEnvironment& declaredVariables() { return m_declaredVariables; }
</span><span class="cx"> VariableEnvironment& lexicalVariables() { return m_lexicalVariables; }
</span><span class="cx"> VariableEnvironment& finalizeLexicalEnvironment()
</span><span class="lines">@@ -323,16 +323,15 @@
</span><span class="cx"> // lexical scope off the stack, we should find which variables are truly captured, and which
</span><span class="cx"> // variable still may be captured in a parent scope.
</span><span class="cx"> if (m_lexicalVariables.size() && m_closedVariableCandidates.size()) {
</span><del>- auto end = m_closedVariableCandidates.end();
- for (auto iter = m_closedVariableCandidates.begin(); iter != end; ++iter)
- m_lexicalVariables.markVariableAsCapturedIfDefined(iter->get());
</del><ins>+ for (UniquedStringImpl* impl : m_closedVariableCandidates)
+ m_lexicalVariables.markVariableAsCapturedIfDefined(impl);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // We can now purge values from the captured candidates because they're captured in this scope.
</span><span class="cx"> {
</span><span class="cx"> for (auto entry : m_lexicalVariables) {
</span><span class="cx"> if (entry.value.isCaptured())
</span><del>- m_closedVariableCandidates.remove(entry.key);
</del><ins>+ m_closedVariableCandidates.remove(entry.key.get());
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="lines">@@ -587,8 +586,8 @@
</span><span class="cx"> // Propagate closed variable candidates downwards within the same function.
</span><span class="cx"> // Cross function captures will be realized via m_usedVariables propagation.
</span><span class="cx"> if (shouldTrackClosedVariables && !nestedScope->m_isFunctionBoundary && nestedScope->m_closedVariableCandidates.size()) {
</span><del>- IdentifierSet::iterator end = nestedScope->m_closedVariableCandidates.end();
- IdentifierSet::iterator begin = nestedScope->m_closedVariableCandidates.begin();
</del><ins>+ auto end = nestedScope->m_closedVariableCandidates.end();
+ auto begin = nestedScope->m_closedVariableCandidates.begin();
</ins><span class="cx"> m_closedVariableCandidates.add(begin, end);
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="lines">@@ -624,11 +623,11 @@
</span><span class="cx"> capturedVariables.add(entry.key);
</span><span class="cx"> return;
</span><span class="cx"> }
</span><del>- for (IdentifierSet::iterator ptr = m_closedVariableCandidates.begin(); ptr != m_closedVariableCandidates.end(); ++ptr) {
</del><ins>+ for (UniquedStringImpl* impl : m_closedVariableCandidates) {
</ins><span class="cx"> // We refer to m_declaredVariables here directly instead of a hasDeclaredVariable because we want to mark the callee as captured.
</span><del>- if (!m_declaredVariables.contains(*ptr))
</del><ins>+ if (!m_declaredVariables.contains(impl))
</ins><span class="cx"> continue;
</span><del>- capturedVariables.add(*ptr);
</del><ins>+ capturedVariables.add(impl);
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx"> void setStrictMode() { m_strictMode = true; }
</span><span class="lines">@@ -636,9 +635,9 @@
</span><span class="cx"> bool isValidStrictMode() const { return m_isValidStrictMode; }
</span><span class="cx"> bool shadowsArguments() const { return m_shadowsArguments; }
</span><span class="cx">
</span><del>- void copyCapturedVariablesToVector(const UniquedStringImplPtrSet& capturedVariables, Vector<RefPtr<UniquedStringImpl>>& vector)
</del><ins>+ void copyCapturedVariablesToVector(const UniquedStringImplPtrSet& usedVariables, Vector<UniquedStringImpl*, 8>& vector)
</ins><span class="cx"> {
</span><del>- for (UniquedStringImpl* impl : capturedVariables) {
</del><ins>+ for (UniquedStringImpl* impl : usedVariables) {
</ins><span class="cx"> if (m_declaredVariables.contains(impl) || m_lexicalVariables.contains(impl))
</span><span class="cx"> continue;
</span><span class="cx"> vector.append(impl);
</span><span class="lines">@@ -740,7 +739,7 @@
</span><span class="cx"> VariableEnvironment m_lexicalVariables;
</span><span class="cx"> Vector<UniquedStringImplPtrSet, 6> m_usedVariables;
</span><span class="cx"> UniquedStringImplPtrSet m_sloppyModeHoistableFunctionCandidates;
</span><del>- IdentifierSet m_closedVariableCandidates;
</del><ins>+ HashSet<UniquedStringImpl*> m_closedVariableCandidates;
</ins><span class="cx"> RefPtr<ModuleScopeData> m_moduleScopeData;
</span><span class="cx"> DeclarationStacks::FunctionStack m_functionDeclarations;
</span><span class="cx"> };
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreparserSourceProviderCacheItemh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/parser/SourceProviderCacheItem.h (199844 => 199845)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/parser/SourceProviderCacheItem.h 2016-04-21 23:28:48 UTC (rev 199844)
+++ trunk/Source/JavaScriptCore/parser/SourceProviderCacheItem.h 2016-04-21 23:30:36 UTC (rev 199845)
</span><span class="lines">@@ -45,8 +45,7 @@
</span><span class="cx"> bool usesEval;
</span><span class="cx"> bool strictMode;
</span><span class="cx"> InnerArrowFunctionCodeFeatures innerArrowFunctionFeatures;
</span><del>- Vector<RefPtr<UniquedStringImpl>> usedVariables;
- Vector<RefPtr<UniquedStringImpl>> writtenVariables;
</del><ins>+ Vector<UniquedStringImpl*, 8> usedVariables;
</ins><span class="cx"> bool isBodyArrowExpression { false };
</span><span class="cx"> JSTokenType tokenType { CLOSEBRACE };
</span><span class="cx"> };
</span><span class="lines">@@ -93,10 +92,8 @@
</span><span class="cx">
</span><span class="cx"> unsigned lastTockenLineStartOffset;
</span><span class="cx"> unsigned usedVariablesCount;
</span><del>- unsigned writtenVariablesCount;
</del><span class="cx">
</span><span class="cx"> UniquedStringImpl** usedVariables() const { return const_cast<UniquedStringImpl**>(m_variables); }
</span><del>- UniquedStringImpl** writtenVariables() const { return const_cast<UniquedStringImpl**>(&m_variables[usedVariablesCount]); }
</del><span class="cx"> bool isBodyArrowExpression;
</span><span class="cx"> JSTokenType tokenType;
</span><span class="cx">
</span><span class="lines">@@ -108,13 +105,13 @@
</span><span class="cx">
</span><span class="cx"> inline SourceProviderCacheItem::~SourceProviderCacheItem()
</span><span class="cx"> {
</span><del>- for (unsigned i = 0; i < usedVariablesCount + writtenVariablesCount; ++i)
</del><ins>+ for (unsigned i = 0; i < usedVariablesCount; ++i)
</ins><span class="cx"> m_variables[i]->deref();
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> inline std::unique_ptr<SourceProviderCacheItem> SourceProviderCacheItem::create(const SourceProviderCacheItemCreationParameters& parameters)
</span><span class="cx"> {
</span><del>- size_t variableCount = parameters.writtenVariables.size() + parameters.usedVariables.size();
</del><ins>+ size_t variableCount = parameters.usedVariables.size();
</ins><span class="cx"> size_t objectSize = sizeof(SourceProviderCacheItem) + sizeof(UniquedStringImpl*) * variableCount;
</span><span class="cx"> void* slot = fastMalloc(objectSize);
</span><span class="cx"> return std::unique_ptr<SourceProviderCacheItem>(new (slot) SourceProviderCacheItem(parameters));
</span><span class="lines">@@ -133,19 +130,13 @@
</span><span class="cx"> , innerArrowFunctionFeatures(parameters.innerArrowFunctionFeatures)
</span><span class="cx"> , lastTockenLineStartOffset(parameters.lastTockenLineStartOffset)
</span><span class="cx"> , usedVariablesCount(parameters.usedVariables.size())
</span><del>- , writtenVariablesCount(parameters.writtenVariables.size())
</del><span class="cx"> , isBodyArrowExpression(parameters.isBodyArrowExpression)
</span><span class="cx"> , tokenType(parameters.tokenType)
</span><span class="cx"> {
</span><del>- unsigned j = 0;
- for (unsigned i = 0; i < usedVariablesCount; ++i, ++j) {
- m_variables[j] = parameters.usedVariables[i].get();
- m_variables[j]->ref();
</del><ins>+ for (unsigned i = 0; i < usedVariablesCount; ++i) {
+ m_variables[i] = parameters.usedVariables[i];
+ m_variables[i]->ref();
</ins><span class="cx"> }
</span><del>- for (unsigned i = 0; i < writtenVariablesCount; ++i, ++j) {
- m_variables[j] = parameters.writtenVariables[i].get();
- m_variables[j]->ref();
- }
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> #if COMPILER(MSVC)
</span></span></pre>
</div>
</div>
</body>
</html>