⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 243869 in webkit


Ignore:
Timestamp:
Apr 4, 2019, 8:41:07 AM (7 years ago)
Author:
Tadeu Zagallo
Message:

Cache bytecode for jsc.cpp helpers and fix CachedStringImpl
​https://bugs.webkit.org/show_bug.cgi?id=196409

Reviewed by Saam Barati.

JSTests:

  • stress/bytecode-cache-cached-string-impl.js: Added.

(f):
(g):

  • stress/bytecode-cache-run-string.js: Added.

Source/JavaScriptCore:

Some of the helpers in jsc.cpp, such as functionRunString, were stll using
using makeSource instead of jscSource, which does not use the ShellSourceProvider
and therefore does not write the bytecode cache to disk.

Changing that revealed a bug in bytecode cache. The Encoder keeps a mapping
of pointers to offsets of already cached objects, in order to avoid caching
the same object twice. Similarly, the Decoder keeps a mapping from offsets
to pointers, in order to avoid creating multiple objects in memory for the
same cached object. The following was happening:
1) A StringImpl* S was cached as CachedPtr<CachedStringImpl> at offset O. We add
an entry in the Encoder mapping that S has already been encoded at O.
2) We cache StringImpl* S again, but now as CachedPtr<CachedUniquedStringImpl>.
We find an entry in the Encoder mapping for S, and return the offset O. However,
the object cached at O is a CachedPtr<CachedStringImpl> (i.e. not Uniqued).

3) When decoding, there are 2 possibilities:
3.1) We find S for the first time through a CachedPtr<CachedStringImpl>. In
this case, everything works as expected since we add an entry in the decoder
mapping from the offset O to the decoded StringImpl* S. The next time we find
S through the uniqued version, we'll return the already decoded S.
3.2) We find S through a CachedPtr<CachedUniquedStringImpl>. Now we have a
problem, since the CachedPtr has the offset of a CachedStringImpl (not uniqued),
which has a different shape and we crash.

We fix this by making CachedStringImpl and CachedUniquedStringImpl share the
same implementation. Since it doesn't matter whether a string is uniqued for
encoding, and we always decode strings as uniqued either way, they can be used
interchangeably.

  • jsc.cpp:

(functionRunString):
(functionLoadString):
(functionDollarAgentStart):
(functionCheckModuleSyntax):
(runInteractive):

  • runtime/CachedTypes.cpp:

(JSC::CachedUniquedStringImplBase::decode const):
(JSC::CachedFunctionExecutable::rareData const):
(JSC::CachedCodeBlock::rareData const):
(JSC::CachedFunctionExecutable::encode):
(JSC::CachedCodeBlock<CodeBlockType>::encode):
(JSC::CachedUniquedStringImpl::encode): Deleted.
(JSC::CachedUniquedStringImpl::decode const): Deleted.
(JSC::CachedStringImpl::encode): Deleted.
(JSC::CachedStringImpl::decode const): Deleted.

Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r243851 r243869  
     12019-04-04  Tadeu Zagallo  <tzagallo@apple.com>
     2
     3        Cache bytecode for jsc.cpp helpers and fix CachedStringImpl
     4        https://bugs.webkit.org/show_bug.cgi?id=196409
     5
     6        Reviewed by Saam Barati.
     7
     8        * stress/bytecode-cache-cached-string-impl.js: Added.
     9        (f):
     10        (g):
     11        * stress/bytecode-cache-run-string.js: Added.
     12
    1132019-04-03  Robin Morisset  <rmorisset@apple.com>
    214

  • trunk/Source/JavaScriptCore/ChangeLog

    r243867 r243869  
     12019-04-04  Tadeu Zagallo  <tzagallo@apple.com>
     2
     3        Cache bytecode for jsc.cpp helpers and fix CachedStringImpl
     4        https://bugs.webkit.org/show_bug.cgi?id=196409
     5
     6        Reviewed by Saam Barati.
     7
     8        Some of the helpers in jsc.cpp, such as `functionRunString`, were stll using
     9        using `makeSource` instead of `jscSource`, which does not use the ShellSourceProvider
     10        and therefore does not write the bytecode cache to disk.
     11
     12        Changing that revealed a bug in bytecode cache. The Encoder keeps a mapping
     13        of pointers to offsets of already cached objects, in order to avoid caching
     14        the same object twice. Similarly, the Decoder keeps a mapping from offsets
     15        to pointers, in order to avoid creating multiple objects in memory for the
     16        same cached object. The following was happening:
     17        1) A StringImpl* S was cached as CachedPtr<CachedStringImpl> at offset O. We add
     18        an entry in the Encoder mapping that S has already been encoded at O.
     19        2) We cache StringImpl* S again, but now as CachedPtr<CachedUniquedStringImpl>.
     20        We find an entry in the Encoder mapping for S, and return the offset O. However,
     21        the object cached at O is a CachedPtr<CachedStringImpl> (i.e. not Uniqued).
     22
     23        3) When decoding, there are 2 possibilities:
     24        3.1) We find S for the first time through a CachedPtr<CachedStringImpl>. In
     25        this case, everything works as expected since we add an entry in the decoder
     26        mapping from the offset O to the decoded StringImpl* S. The next time we find
     27        S through the uniqued version, we'll return the already decoded S.
     28        3.2) We find S through a CachedPtr<CachedUniquedStringImpl>. Now we have a
     29        problem, since the CachedPtr has the offset of a CachedStringImpl (not uniqued),
     30        which has a different shape and we crash.
     31
     32        We fix this by making CachedStringImpl and CachedUniquedStringImpl share the
     33        same implementation. Since it doesn't matter whether a string is uniqued for
     34        encoding, and we always decode strings as uniqued either way, they can be used
     35        interchangeably.
     36
     37        * jsc.cpp:
     38        (functionRunString):
     39        (functionLoadString):
     40        (functionDollarAgentStart):
     41        (functionCheckModuleSyntax):
     42        (runInteractive):
     43        * runtime/CachedTypes.cpp:
     44        (JSC::CachedUniquedStringImplBase::decode const):
     45        (JSC::CachedFunctionExecutable::rareData const):
     46        (JSC::CachedCodeBlock::rareData const):
     47        (JSC::CachedFunctionExecutable::encode):
     48        (JSC::CachedCodeBlock<CodeBlockType>::encode):
     49        (JSC::CachedUniquedStringImpl::encode): Deleted.
     50        (JSC::CachedUniquedStringImpl::decode const): Deleted.
     51        (JSC::CachedStringImpl::encode): Deleted.
     52        (JSC::CachedStringImpl::decode const): Deleted.
     53
    1542019-04-04  Tadeu Zagallo  <tzagallo@apple.com>
    255

  • trunk/Source/JavaScriptCore/jsc.cpp

    r243312 r243869  
    14601460
    14611461    NakedPtr<Exception> exception;
    1462     evaluate(globalObject->globalExec(), makeSource(source, exec->callerSourceOrigin()), JSValue(), exception);
     1462    evaluate(globalObject->globalExec(), jscSource(source, exec->callerSourceOrigin()), JSValue(), exception);
    14631463
    14641464    if (exception) {
    … …  
    15001500
    15011501    NakedPtr<Exception> evaluationException;
    1502     JSValue result = evaluate(globalObject->globalExec(), makeSource(sourceCode, exec->callerSourceOrigin()), JSValue(), evaluationException);
     1502    JSValue result = evaluate(globalObject->globalExec(), jscSource(sourceCode, exec->callerSourceOrigin()), JSValue(), evaluationException);
    15031503    if (evaluationException)
    15041504        throwException(exec, scope, evaluationException);
    … …  
    18441844                    NakedPtr<Exception> evaluationException;
    18451845                    JSValue result;
    1846                     result = evaluate(globalObject->globalExec(), makeSource(sourceCode, SourceOrigin("worker"_s)), JSValue(), evaluationException);
     1846                    result = evaluate(globalObject->globalExec(), jscSource(sourceCode, SourceOrigin("worker"_s)), JSValue(), evaluationException);
    18471847                    if (evaluationException)
    18481848                        result = evaluationException->value();
    … …  
    21862186
    21872187    ParserError error;
    2188     bool validSyntax = checkModuleSyntax(exec, makeSource(source, { }, URL(), TextPosition(), SourceProviderSourceType::Module), error);
     2188    bool validSyntax = checkModuleSyntax(exec, jscSource(source, { }, URL(), TextPosition(), SourceProviderSourceType::Module), error);
    21892189    RETURN_IF_EXCEPTION(scope, encodedJSValue());
    21902190    stopWatch.stop();
    … …  
    26252625            source = source + String::fromUTF8(line);
    26262626            source = source + '\n';
    2627             checkSyntax(vm, makeSource(source, sourceOrigin), error);
     2627            checkSyntax(vm, jscSource(source, sourceOrigin), error);
    26282628            if (!line[0]) {
    26292629                free(line);
    … …  
    26412641       
    26422642        NakedPtr<Exception> evaluationException;
    2643         JSValue returnValue = evaluate(globalObject->globalExec(), makeSource(source, sourceOrigin), JSValue(), evaluationException);
     2643        JSValue returnValue = evaluate(globalObject->globalExec(), jscSource(source, sourceOrigin), JSValue(), evaluationException);
    26442644#else
    26452645        printf("%s", interactivePrompt);

  • trunk/Source/JavaScriptCore/runtime/CachedTypes.cpp

    r243867 r243869  
    576576};
    577577
    578 class CachedUniquedStringImpl : public VariableLengthObject<UniquedStringImpl> {
     578template<typename T>
     579class CachedUniquedStringImplBase : public VariableLengthObject<T> {
    579580public:
    580581    void encode(Encoder& encoder, const StringImpl& string)
    … …  
    632633
    633634        if (m_is8Bit)
    634             return create(this->buffer<LChar>());
    635         return create(this->buffer<UChar>());
     635            return create(this->template buffer<LChar>());
     636        return create(this->template buffer<UChar>());
    636637    }
    637638
    … …  
    643644};
    644645
    645 class CachedStringImpl : public VariableLengthObject<StringImpl> {
    646 public:
    647     void encode(Encoder& encoder, const StringImpl& impl)
    648     {
    649         m_uniquedStringImpl.encode(encoder, impl);
    650     }
    651 
    652     StringImpl* decode(Decoder& decoder) const
    653     {
    654         return m_uniquedStringImpl.decode(decoder);
    655     }
    656 
    657 private:
    658     CachedUniquedStringImpl m_uniquedStringImpl;
    659 };
     646class CachedUniquedStringImpl : public CachedUniquedStringImplBase<UniquedStringImpl> { };
     647class CachedStringImpl : public CachedUniquedStringImplBase<StringImpl> { };
    660648
    661649class CachedString : public VariableLengthObject<String> {
    … …  
    15731561    Identifier inferredName(Decoder& decoder) const { return m_inferredName.decode(decoder); }
    15741562
    1575     UnlinkedFunctionExecutable::RareData* rareData(Decoder& decoder) const { return m_rareData.decodeAsPtr(decoder); }
     1563    UnlinkedFunctionExecutable::RareData* rareData(Decoder& decoder) const { return m_rareData.decode(decoder); }
    15761564
    15771565    const CachedWriteBarrier<CachedFunctionCodeBlock, UnlinkedFunctionCodeBlock>& unlinkedCodeBlockForCall() const { return m_unlinkedCodeBlockForCall; }
    … …  
    16031591    unsigned m_derivedContextType: 2;
    16041592
    1605     CachedOptional<CachedFunctionExecutableRareData> m_rareData;
     1593    CachedPtr<CachedFunctionExecutableRareData> m_rareData;
    16061594
    16071595    CachedIdentifier m_name;
    … …  
    16561644    unsigned codeType() const { return m_codeType; }
    16571645
    1658     UnlinkedCodeBlock::RareData* rareData(Decoder& decoder) const { return m_rareData.decodeAsPtr(decoder); }
     1646    UnlinkedCodeBlock::RareData* rareData(Decoder& decoder) const { return m_rareData.decode(decoder); }
    16591647
    16601648private:
    … …  
    16911679    CachedMetadataTable m_metadata;
    16921680
    1693     CachedOptional<CachedCodeBlockRareData> m_rareData;
     1681    CachedPtr<CachedCodeBlockRareData> m_rareData;
    16941682
    16951683    CachedString m_sourceURLDirective;
    … …  
    19561944    m_derivedContextType = executable.m_derivedContextType;
    19571945
    1958     m_rareData.encode(encoder, executable.m_rareData);
     1946    m_rareData.encode(encoder, executable.m_rareData.get());
    19591947
    19601948    m_name.encode(encoder, executable.name());
    … …  
    20612049
    20622050    m_metadata.encode(encoder, codeBlock.m_metadata.get());
    2063     m_rareData.encode(encoder, codeBlock.m_rareData);
     2051    m_rareData.encode(encoder, codeBlock.m_rareData.get());
    20642052
    20652053    m_sourceURLDirective.encode(encoder, codeBlock.m_sourceURLDirective.impl());
Note: See TracChangeset for help on using the changeset viewer.