Skip to content

Commit 75b131e

Browse files
saambaratisaambarati
committed
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. Canonical link: https://commits.webkit.org/174980@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@199845 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 10d719f commit 75b131e

4 files changed

Lines changed: 56 additions & 31 deletions

File tree

‎Source/JavaScriptCore/ChangeLog‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,38 @@
1+
2016-04-21 Saam barati <sbarati@apple.com>
2+
3+
Remove some unnecessary RefPtrs in the parser
4+
https://bugs.webkit.org/show_bug.cgi?id=156865
5+
6+
Reviewed by Filip Pizlo.
7+
8+
The IdentifierArena or the SourceProviderCacheItem will own these UniquedStringImpls
9+
while we are using them. There is no need for us to reference count them.
10+
11+
This might be a 0.5% speedup on octane code-load.
12+
13+
* parser/Parser.cpp:
14+
(JSC::Parser<LexerType>::parseInner):
15+
* parser/Parser.h:
16+
(JSC::Scope::setIsLexicalScope):
17+
(JSC::Scope::isLexicalScope):
18+
(JSC::Scope::closedVariableCandidates):
19+
(JSC::Scope::declaredVariables):
20+
(JSC::Scope::lexicalVariables):
21+
(JSC::Scope::finalizeLexicalEnvironment):
22+
(JSC::Scope::computeLexicallyCapturedVariablesAndPurgeCandidates):
23+
(JSC::Scope::collectFreeVariables):
24+
(JSC::Scope::getCapturedVars):
25+
(JSC::Scope::setStrictMode):
26+
(JSC::Scope::isValidStrictMode):
27+
(JSC::Scope::shadowsArguments):
28+
(JSC::Scope::copyCapturedVariablesToVector):
29+
* parser/SourceProviderCacheItem.h:
30+
(JSC::SourceProviderCacheItem::usedVariables):
31+
(JSC::SourceProviderCacheItem::~SourceProviderCacheItem):
32+
(JSC::SourceProviderCacheItem::create):
33+
(JSC::SourceProviderCacheItem::SourceProviderCacheItem):
34+
(JSC::SourceProviderCacheItem::writtenVariables): Deleted.
35+
136
2016-04-21 Filip Pizlo <fpizlo@apple.com>
237

338
PolymorphicAccess adds sizeof(CallerFrameAndPC) rather than subtracting it when calculating stack height

‎Source/JavaScriptCore/parser/Parser.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -318,10 +318,10 @@ String Parser<LexerType>::parseInner(const Identifier& calleeName, SourceParseMo
318318
#ifndef NDEBUG
319319
if (m_parsingBuiltin && isProgramParseMode(parseMode)) {
320320
VariableEnvironment& lexicalVariables = scope->lexicalVariables();
321-
const IdentifierSet& closedVariableCandidates = scope->closedVariableCandidates();
321+
const HashSet<UniquedStringImpl*>& closedVariableCandidates = scope->closedVariableCandidates();
322322
const BuiltinNames& builtinNames = m_vm->propertyNames->builtinNames();
323-
for (const RefPtr<UniquedStringImpl>& candidate : closedVariableCandidates) {
324-
if (!lexicalVariables.contains(candidate) && !varDeclarations.contains(candidate) && !builtinNames.isPrivateName(*candidate.get())) {
323+
for (UniquedStringImpl* candidate : closedVariableCandidates) {
324+
if (!lexicalVariables.contains(candidate) && !varDeclarations.contains(candidate) && !builtinNames.isPrivateName(*candidate)) {
325325
dataLog("Bad global capture in builtin: '", candidate, "'\n");
326326
dataLog(m_source->view());
327327
CRASH();

‎Source/JavaScriptCore/parser/Parser.h‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ struct Scope {
297297
}
298298
bool isLexicalScope() { return m_isLexicalScope; }
299299

300-
const IdentifierSet& closedVariableCandidates() const { return m_closedVariableCandidates; }
300+
const HashSet<UniquedStringImpl*>& closedVariableCandidates() const { return m_closedVariableCandidates; }
301301
VariableEnvironment& declaredVariables() { return m_declaredVariables; }
302302
VariableEnvironment& lexicalVariables() { return m_lexicalVariables; }
303303
VariableEnvironment& finalizeLexicalEnvironment()
@@ -323,16 +323,15 @@ struct Scope {
323323
// lexical scope off the stack, we should find which variables are truly captured, and which
324324
// variable still may be captured in a parent scope.
325325
if (m_lexicalVariables.size() && m_closedVariableCandidates.size()) {
326-
auto end = m_closedVariableCandidates.end();
327-
for (auto iter = m_closedVariableCandidates.begin(); iter != end; ++iter)
328-
m_lexicalVariables.markVariableAsCapturedIfDefined(iter->get());
326+
for (UniquedStringImpl* impl : m_closedVariableCandidates)
327+
m_lexicalVariables.markVariableAsCapturedIfDefined(impl);
329328
}
330329

331330
// We can now purge values from the captured candidates because they're captured in this scope.
332331
{
333332
for (auto entry : m_lexicalVariables) {
334333
if (entry.value.isCaptured())
335-
m_closedVariableCandidates.remove(entry.key);
334+
m_closedVariableCandidates.remove(entry.key.get());
336335
}
337336
}
338337
}
@@ -587,8 +586,8 @@ struct Scope {
587586
// Propagate closed variable candidates downwards within the same function.
588587
// Cross function captures will be realized via m_usedVariables propagation.
589588
if (shouldTrackClosedVariables && !nestedScope->m_isFunctionBoundary && nestedScope->m_closedVariableCandidates.size()) {
590-
IdentifierSet::iterator end = nestedScope->m_closedVariableCandidates.end();
591-
IdentifierSet::iterator begin = nestedScope->m_closedVariableCandidates.begin();
589+
auto end = nestedScope->m_closedVariableCandidates.end();
590+
auto begin = nestedScope->m_closedVariableCandidates.begin();
592591
m_closedVariableCandidates.add(begin, end);
593592
}
594593
}
@@ -624,21 +623,21 @@ struct Scope {
624623
capturedVariables.add(entry.key);
625624
return;
626625
}
627-
for (IdentifierSet::iterator ptr = m_closedVariableCandidates.begin(); ptr != m_closedVariableCandidates.end(); ++ptr) {
626+
for (UniquedStringImpl* impl : m_closedVariableCandidates) {
628627
// We refer to m_declaredVariables here directly instead of a hasDeclaredVariable because we want to mark the callee as captured.
629-
if (!m_declaredVariables.contains(*ptr))
628+
if (!m_declaredVariables.contains(impl))
630629
continue;
631-
capturedVariables.add(*ptr);
630+
capturedVariables.add(impl);
632631
}
633632
}
634633
void setStrictMode() { m_strictMode = true; }
635634
bool strictMode() const { return m_strictMode; }
636635
bool isValidStrictMode() const { return m_isValidStrictMode; }
637636
bool shadowsArguments() const { return m_shadowsArguments; }
638637

639-
void copyCapturedVariablesToVector(const UniquedStringImplPtrSet& capturedVariables, Vector<RefPtr<UniquedStringImpl>>& vector)
638+
void copyCapturedVariablesToVector(const UniquedStringImplPtrSet& usedVariables, Vector<UniquedStringImpl*, 8>& vector)
640639
{
641-
for (UniquedStringImpl* impl : capturedVariables) {
640+
for (UniquedStringImpl* impl : usedVariables) {
642641
if (m_declaredVariables.contains(impl) || m_lexicalVariables.contains(impl))
643642
continue;
644643
vector.append(impl);
@@ -740,7 +739,7 @@ struct Scope {
740739
VariableEnvironment m_lexicalVariables;
741740
Vector<UniquedStringImplPtrSet, 6> m_usedVariables;
742741
UniquedStringImplPtrSet m_sloppyModeHoistableFunctionCandidates;
743-
IdentifierSet m_closedVariableCandidates;
742+
HashSet<UniquedStringImpl*> m_closedVariableCandidates;
744743
RefPtr<ModuleScopeData> m_moduleScopeData;
745744
DeclarationStacks::FunctionStack m_functionDeclarations;
746745
};

‎Source/JavaScriptCore/parser/SourceProviderCacheItem.h‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@ struct SourceProviderCacheItemCreationParameters {
4545
bool usesEval;
4646
bool strictMode;
4747
InnerArrowFunctionCodeFeatures innerArrowFunctionFeatures;
48-
Vector<RefPtr<UniquedStringImpl>> usedVariables;
49-
Vector<RefPtr<UniquedStringImpl>> writtenVariables;
48+
Vector<UniquedStringImpl*, 8> usedVariables;
5049
bool isBodyArrowExpression { false };
5150
JSTokenType tokenType { CLOSEBRACE };
5251
};
@@ -93,10 +92,8 @@ class SourceProviderCacheItem {
9392

9493
unsigned lastTockenLineStartOffset;
9594
unsigned usedVariablesCount;
96-
unsigned writtenVariablesCount;
9795

9896
UniquedStringImpl** usedVariables() const { return const_cast<UniquedStringImpl**>(m_variables); }
99-
UniquedStringImpl** writtenVariables() const { return const_cast<UniquedStringImpl**>(&m_variables[usedVariablesCount]); }
10097
bool isBodyArrowExpression;
10198
JSTokenType tokenType;
10299

@@ -108,13 +105,13 @@ class SourceProviderCacheItem {
108105

109106
inline SourceProviderCacheItem::~SourceProviderCacheItem()
110107
{
111-
for (unsigned i = 0; i < usedVariablesCount + writtenVariablesCount; ++i)
108+
for (unsigned i = 0; i < usedVariablesCount; ++i)
112109
m_variables[i]->deref();
113110
}
114111

115112
inline std::unique_ptr<SourceProviderCacheItem> SourceProviderCacheItem::create(const SourceProviderCacheItemCreationParameters& parameters)
116113
{
117-
size_t variableCount = parameters.writtenVariables.size() + parameters.usedVariables.size();
114+
size_t variableCount = parameters.usedVariables.size();
118115
size_t objectSize = sizeof(SourceProviderCacheItem) + sizeof(UniquedStringImpl*) * variableCount;
119116
void* slot = fastMalloc(objectSize);
120117
return std::unique_ptr<SourceProviderCacheItem>(new (slot) SourceProviderCacheItem(parameters));
@@ -133,18 +130,12 @@ inline SourceProviderCacheItem::SourceProviderCacheItem(const SourceProviderCach
133130
, innerArrowFunctionFeatures(parameters.innerArrowFunctionFeatures)
134131
, lastTockenLineStartOffset(parameters.lastTockenLineStartOffset)
135132
, usedVariablesCount(parameters.usedVariables.size())
136-
, writtenVariablesCount(parameters.writtenVariables.size())
137133
, isBodyArrowExpression(parameters.isBodyArrowExpression)
138134
, tokenType(parameters.tokenType)
139135
{
140-
unsigned j = 0;
141-
for (unsigned i = 0; i < usedVariablesCount; ++i, ++j) {
142-
m_variables[j] = parameters.usedVariables[i].get();
143-
m_variables[j]->ref();
144-
}
145-
for (unsigned i = 0; i < writtenVariablesCount; ++i, ++j) {
146-
m_variables[j] = parameters.writtenVariables[i].get();
147-
m_variables[j]->ref();
136+
for (unsigned i = 0; i < usedVariablesCount; ++i) {
137+
m_variables[i] = parameters.usedVariables[i];
138+
m_variables[i]->ref();
148139
}
149140
}
150141

0 commit comments

Comments
 (0)