Bug 1008032 - Make ScriptSource a single threaded data structure, clean it up some, r=luke.

js/src/jsscript.cpp

@@ -1344,29 +1344,6 @@ ScriptSourceObject::create(ExclusiveContext *cx, ScriptSource *source,
     return sourceObject;
 }
 
-static const unsigned char emptySource[] = "";
-
-/* Adjust the amount of memory this script source uses for source data,
-   reallocating if needed. */
-bool
-ScriptSource::adjustDataSize(size_t nbytes)
-{
-    // Allocating 0 bytes has undefined behavior, so special-case it.
-    if (nbytes == 0) {
-        if (data.compressed != emptySource)
-            js_free(data.compressed);
-        data.compressed = const_cast<unsigned char *>(emptySource);
-        return true;
-    }
-
-    // |data.compressed| can be nullptr.
-    void *buf = js_realloc(data.compressed, nbytes);
-    if (!buf && data.compressed != emptySource)
-        js_free(data.compressed);
-    data.compressed = static_cast<unsigned char *>(buf);
-    return !!data.compressed;
-}
-
 /* static */ bool
 JSScript::loadSource(JSContext *cx, ScriptSource *ss, bool *worked)
 {
@@ -1520,12 +1497,12 @@ SourceDataCache::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf)
 const jschar *
 ScriptSource::chars(JSContext *cx, SourceDataCache::AutoHoldEntry &holder)
 {
-    if (const jschar *chars = getOffThreadCompressionChars(cx))
-        return chars;
-    JS_ASSERT(ready());
+    switch (dataType) {
+      case DataUncompressed:
+        return uncompressedChars();
 
+      case DataCompressed: {
 #ifdef USE_ZLIB
-    if (compressed()) {
         if (const jschar *decompressed = cx->runtime()->sourceDataCache.lookup(this, holder))
             return decompressed;
 
@@ -1534,7 +1511,7 @@ ScriptSource::chars(JSContext *cx, SourceDataCache::AutoHoldEntry &holder)
         if (!decompressed)
             return nullptr;
 
-        if (!DecompressString(data.compressed, compressedLength_,
+        if (!DecompressString((const unsigned char *) compressedData(), compressedBytes(),
                               reinterpret_cast<unsigned char *>(decompressed), nbytes)) {
             JS_ReportOutOfMemory(cx);
             js_free(decompressed);
@@ -1550,9 +1527,14 @@ ScriptSource::chars(JSContext *cx, SourceDataCache::AutoHoldEntry &holder)
         }
 
         return decompressed;
-    }
+#else
+        MOZ_CRASH();
 #endif
-    return data.source;
+      }
+
+      default:
+        MOZ_CRASH();
+    }
 }
 
 JSFlatString *
@@ -1566,14 +1548,57 @@ ScriptSource::substring(JSContext *cx, uint32_t start, uint32_t stop)
     return js_NewStringCopyN<CanGC>(cx, chars + start, stop - start);
 }
 
+void
+ScriptSource::setSource(const jschar *chars, size_t length, bool ownsChars /* = true */)
+{
+    JS_ASSERT(dataType == DataMissing);
+
+    dataType = DataUncompressed;
+    data.uncompressed.chars = chars;
+    data.uncompressed.ownsChars = ownsChars;
+
+    length_ = length;
+}
+
+void
+ScriptSource::setCompressedSource(void *raw, size_t nbytes)
+{
+    JS_ASSERT(dataType == DataMissing || dataType == DataUncompressed);
+    if (dataType == DataUncompressed && ownsUncompressedChars())
+        js_free(const_cast<jschar *>(uncompressedChars()));
+
+    dataType = DataCompressed;
+    data.compressed.raw = raw;
+    data.compressed.nbytes = nbytes;
+}
+
+bool
+ScriptSource::ensureOwnsSource(ExclusiveContext *cx)
+{
+    JS_ASSERT(dataType == DataUncompressed);
+    if (ownsUncompressedChars())
+        return true;
+
+    jschar *uncompressed = (jschar *) cx->malloc_(sizeof(jschar) * Max<size_t>(length_, 1));
+    if (!uncompressed)
+        return false;
+    PodCopy(uncompressed, uncompressedChars(), length_);
+
+    data.uncompressed.chars = uncompressed;
+    data.uncompressed.ownsChars = true;
+    return true;
+}
+
 bool
 ScriptSource::setSourceCopy(ExclusiveContext *cx, SourceBufferHolder &srcBuf,
                             bool argumentsNotIncluded, SourceCompressionTask *task)
 {
     JS_ASSERT(!hasSourceData());
-    length_ = srcBuf.length();
     argumentsNotIncluded_ = argumentsNotIncluded;
 
+    bool owns = srcBuf.ownsChars();
+    setSource(owns ? srcBuf.take() : srcBuf.get(), srcBuf.length(), owns);
+
     // There are several cases where source compression is not a good idea:
     //  - If the script is tiny, then compression will save little or no space.
     //  - If the script is enormous, then decompression can take seconds. With
@@ -1606,102 +1631,90 @@ ScriptSource::setSourceCopy(ExclusiveContext *cx, SourceBufferHolder &srcBuf,
     const size_t HUGE_SCRIPT = 5 * 1024 * 1024;
     if (TINY_SCRIPT <= srcBuf.length() && srcBuf.length() < HUGE_SCRIPT && canCompressOffThread) {
         task->ss = this;
-        task->chars = srcBuf.get();
-        ready_ = false;
         if (!StartOffThreadCompression(cx, task))
             return false;
-    } else if (srcBuf.ownsChars()) {
-        data.source = srcBuf.take();
-    } else {
-        if (!adjustDataSize(sizeof(jschar) * srcBuf.length()))
-            return false;
-        PodCopy(data.source, srcBuf.get(), length_);
+    } else if (!ensureOwnsSource(cx)) {
+        return false;
     }
 
     return true;
 }
 
-void
-ScriptSource::setSource(const jschar *src, size_t length)
-{
-    JS_ASSERT(!hasSourceData());
-    length_ = length;
-    JS_ASSERT(!argumentsNotIncluded_);
-    data.source = const_cast<jschar *>(src);
-}
-
-bool
+SourceCompressionTask::ResultType
 SourceCompressionTask::work()
 {
-    // A given compression token can be compressed on any thread, and the ss
-    // not being ready indicates to other threads that its fields might change
-    // with no lock held.
-    JS_ASSERT(!ss->ready());
-
-    size_t compressedLength = 0;
-    size_t nbytes = sizeof(jschar) * ss->length_;
-
-    // Memory allocation functions on JSRuntime and JSContext are not
-    // threadsafe. We have to use the js_* variants.
-
 #ifdef USE_ZLIB
     // Try to keep the maximum memory usage down by only allocating half the
     // size of the string, first.
-    size_t firstSize = nbytes / 2;
-    if (!ss->adjustDataSize(firstSize))
-        return false;
-    Compressor comp(reinterpret_cast<const unsigned char *>(chars), nbytes);
+    size_t inputBytes = ss->length() * sizeof(jschar);
+    size_t firstSize = inputBytes / 2;
+    compressed = js_malloc(firstSize);
+    if (!compressed)
+        return OOM;
+
+    Compressor comp(reinterpret_cast<const unsigned char *>(ss->uncompressedChars()), inputBytes);
     if (!comp.init())
-        return false;
-    comp.setOutput(ss->data.compressed, firstSize);
-    bool cont = !abort_;
+        return OOM;
+
+    comp.setOutput((unsigned char *) compressed, firstSize);
+    bool cont = true;
     while (cont) {
+        if (abort_)
+            return Aborted;
+
         switch (comp.compressMore()) {
           case Compressor::CONTINUE:
             break;
           case Compressor::MOREOUTPUT: {
-            if (comp.outWritten() == nbytes) {
-                cont = false;
-                break;
+            if (comp.outWritten() == inputBytes) {
+                // The compressed string is longer than the original string.
+                return Aborted;
             }
 
             // The compressed output is greater than half the size of the
             // original string. Reallocate to the full size.
-            if (!ss->adjustDataSize(nbytes))
-                return false;
-            comp.setOutput(ss->data.compressed, nbytes);
+            compressed = js_realloc(compressed, inputBytes);
+            if (!compressed)
+                return OOM;
+
+            comp.setOutput((unsigned char *) compressed, inputBytes);
             break;
           }
           case Compressor::DONE:
             cont = false;
             break;
           case Compressor::OOM:
-            return false;
+            return OOM;
         }
-        cont = cont && !abort_;
     }
-    compressedLength = comp.outWritten();
-    if (abort_ || compressedLength == nbytes)
-        compressedLength = 0;
+    compressedBytes = comp.outWritten();
+#else
+    MOZ_CRASH();
 #endif
 
-    if (compressedLength == 0) {
-        if (!ss->adjustDataSize(nbytes))
-            return false;
-        PodCopy(ss->data.source, chars, ss->length());
-    } else {
-        // Shrink the buffer to the size of the compressed data. Shouldn't fail.
-        JS_ALWAYS_TRUE(ss->adjustDataSize(compressedLength));
-    }
-    ss->compressedLength_ = compressedLength;
-    return true;
+    // Shrink the buffer to the size of the compressed data.
+    if (void *newCompressed = js_realloc(compressed, compressedBytes))
+        compressed = newCompressed;
+
+    return Success;
 }
 
-void
-ScriptSource::destroy()
+ScriptSource::~ScriptSource()
 {
-    JS_ASSERT(ready());
-    adjustDataSize(0);
+    switch (dataType) {
+      case DataUncompressed:
+        if (ownsUncompressedChars())
+            js_free(const_cast<jschar *>(uncompressedChars()));
+        break;
+
+      case DataCompressed:
+        js_free(compressedData());
+        break;
+
+      default:
+        break;
+    }
+
     if (introducerFilename_ != filename_)
         js_free(introducerFilename_);
     js_free(filename_);
@@ -1709,20 +1722,16 @@ ScriptSource::destroy()
     js_free(sourceMapURL_);
     if (originPrincipals_)
         JS_DropPrincipals(TlsPerThreadData.get()->runtimeFromMainThread(), originPrincipals_);
-    ready_ = false;
-    js_free(this);
 }
 
 void
 ScriptSource::addSizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf,
                                      JS::ScriptSourceInfo *info) const
 {
-    if (ready() && data.compressed != emptySource) {
-        if (compressed())
-            info->compressed += mallocSizeOf(data.compressed);
-        else
-            info->uncompressed += mallocSizeOf(data.source);
-    }
+    if (dataType == DataUncompressed && ownsUncompressedChars())
+        info->uncompressed += mallocSizeOf(uncompressedChars());
+    else if (dataType == DataCompressed)
+        info->compressed += mallocSizeOf(compressedData());
     info->misc += mallocSizeOf(this) + mallocSizeOf(filename_);
     info->numScripts++;
 }
@@ -1741,35 +1750,40 @@ ScriptSource::performXDR(XDRState<mode> *xdr)
     sourceRetrievable_ = retrievable;
 
     if (hasSource && !sourceRetrievable_) {
-        // Only set members when we know decoding cannot fail. This prevents the
-        // script source from being partially initialized.
-        uint32_t length = length_;
-        if (!xdr->codeUint32(&length))
+        if (!xdr->codeUint32(&length_))
             return false;
 
-        uint32_t compressedLength = compressedLength_;
+        uint32_t compressedLength = (dataType == DataCompressed) ? compressedBytes() : 0;
         if (!xdr->codeUint32(&compressedLength))
             return false;
 
-        uint8_t argumentsNotIncluded = argumentsNotIncluded_;
-        if (!xdr->codeUint8(&argumentsNotIncluded))
-            return false;
+        {
+            uint8_t argumentsNotIncluded;
+            if (mode == XDR_ENCODE)
+                argumentsNotIncluded = argumentsNotIncluded_;
+            if (!xdr->codeUint8(&argumentsNotIncluded))
+                return false;
+            if (mode == XDR_DECODE)
+                argumentsNotIncluded_ = argumentsNotIncluded;
+        }
 
-        size_t byteLen = compressedLength ? compressedLength : (length * sizeof(jschar));
+        size_t byteLen = compressedLength ? compressedLength : (length_ * sizeof(jschar));
         if (mode == XDR_DECODE) {
-            if (!adjustDataSize(byteLen))
+            void *p = xdr->cx()->malloc_(Max<size_t>(byteLen, 1));
+            if (!p || !xdr->codeBytes(p, byteLen)) {
+                js_free(p);
+                return false;
+            }
+
+            if (compressedLength)
+                setCompressedSource(p, compressedLength);
+            else
+                setSource((const jschar *) p, length_);
+        } else {
+            void *p = compressedLength ? compressedData() : (void *) uncompressedChars();
+            if (!xdr->codeBytes(p, byteLen))
                 return false;
         }
-        if (!xdr->codeBytes(data.compressed, byteLen)) {
-            if (mode == XDR_DECODE) {
-                js_free(data.compressed);
-                data.compressed = nullptr;
-            }
-            return false;
-        }
-        length_ = length;
-        compressedLength_ = compressedLength;
-        argumentsNotIncluded_ = argumentsNotIncluded;
     }
 
     uint8_t haveSourceMap = hasSourceMapURL();
@@ -1834,9 +1848,6 @@ ScriptSource::performXDR(XDRState<mode> *xdr)
             return false;
     }
 
-    if (mode == XDR_DECODE)
-        ready_ = true;
-
     return true;
 }
 
@@ -1983,6 +1994,16 @@ ScriptSource::sourceMapURL()
     return sourceMapURL_;
 }
 
+size_t
+ScriptSource::computedSizeOfData() const
+{
+    if (dataType == DataUncompressed && ownsUncompressedChars())
+        return sizeof(jschar) * length_;
+    if (dataType == DataCompressed)
+        return compressedBytes();
+    return 0;
+}
+
 /*
  * Shared script data management.
  */

js/src/jsscript.h

@@ -386,30 +386,32 @@ class ScriptSource
 {
     friend class SourceCompressionTask;
 
-    // A note on concurrency:
-    //
-    // The source may be compressed by a worker thread during parsing. (See
-    // SourceCompressionTask.) When compression is running in the background,
-    // ready() returns false. The compression thread touches the |data| union
-    // and |compressedLength_|. Therefore, it is not safe to read these members
-    // unless ready() is true. With that said, users of the public ScriptSource
-    // API should be fine.
+    uint32_t refs;
+
+    // Note: while ScriptSources may be compressed off thread, they are only
+    // modified by the main thread, and all members are always safe to access
+    // on the main thread.
+
+    // Indicate which field in the |data| union is active.
+    enum {
+        DataMissing,
+        DataUncompressed,
+        DataCompressed
+    } dataType;
 
     union {
-        // Before setSourceCopy or setSource are successfully called, this union
-        // has a nullptr pointer. When the script source is ready,
-        // compressedLength_ != 0 implies compressed holds the compressed data;
-        // otherwise, source holds the uncompressed source. There is a special
-        // pointer |emptySource| for source code for length 0.
-        //
-        // The only function allowed to malloc, realloc, or free the pointers in
-        // this union is adjustDataSize(). Don't do it elsewhere.
-        jschar *source;
-        unsigned char *compressed;
+        struct {
+            const jschar *chars;
+            bool ownsChars;
+        } uncompressed;
+
+        struct {
+            void *raw;
+            size_t nbytes;
+        } compressed;
     } data;
-    uint32_t refs;
+
     uint32_t length_;
-    uint32_t compressedLength_;
     char *filename_;
     jschar *displayURL_;
     jschar *sourceMapURL_;
@@ -446,14 +448,13 @@ class ScriptSource
     // possible to get source at all.
     bool sourceRetrievable_:1;
     bool argumentsNotIncluded_:1;
-    bool ready_:1;
     bool hasIntroductionOffset_:1;
 
   public:
     explicit ScriptSource()
       : refs(0),
+        dataType(DataMissing),
         length_(0),
-        compressedLength_(0),
         filename_(nullptr),
         displayURL_(nullptr),
         sourceMapURL_(nullptr),
@@ -463,28 +464,25 @@ class ScriptSource
         introductionType_(nullptr),
         sourceRetrievable_(false),
         argumentsNotIncluded_(false),
-        ready_(true),
         hasIntroductionOffset_(false)
     {
-        data.source = nullptr;
     }
+    ~ScriptSource();
     void incref() { refs++; }
     void decref() {
         JS_ASSERT(refs != 0);
         if (--refs == 0)
-            destroy();
+            js_delete(this);
     }
     bool initFromOptions(ExclusiveContext *cx, const ReadOnlyCompileOptions &options);
     bool setSourceCopy(ExclusiveContext *cx,
                        JS::SourceBufferHolder &srcBuf,
                        bool argumentsNotIncluded,
                        SourceCompressionTask *tok);
-    void setSource(const jschar *src, size_t length);
-    bool ready() const { return ready_; }
     void setSourceRetrievable() { sourceRetrievable_ = true; }
     bool sourceRetrievable() const { return sourceRetrievable_; }
-    bool hasSourceData() const { return !ready() || !!data.source; }
-    uint32_t length() const {
+    bool hasSourceData() const { return dataType != DataMissing; }
+    size_t length() const {
         JS_ASSERT(hasSourceData());
         return length_;
     }
@@ -497,6 +495,30 @@ class ScriptSource
     void addSizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf,
                                 JS::ScriptSourceInfo *info) const;
 
+    const jschar *uncompressedChars() const {
+        JS_ASSERT(dataType == DataUncompressed);
+        return data.uncompressed.chars;
+    }
+
+    bool ownsUncompressedChars() const {
+        JS_ASSERT(dataType == DataUncompressed);
+        return data.uncompressed.ownsChars;
+    }
+
+    void *compressedData() const {
+        JS_ASSERT(dataType == DataCompressed);
+        return data.compressed.raw;
+    }
+
+    size_t compressedBytes() const {
+        JS_ASSERT(dataType == DataCompressed);
+        return data.compressed.nbytes;
+    }
+
+    void setSource(const jschar *chars, size_t length, bool ownsChars = true);
+    void setCompressedSource(void *raw, size_t nbytes);
+    bool ensureOwnsSource(ExclusiveContext *cx);
+
     // XDR handling
     template <XDRMode mode>
     bool performXDR(XDRState<mode> *xdr);
@@ -541,13 +563,7 @@ class ScriptSource
     }
 
   private:
-    void destroy();
-    bool compressed() const { return compressedLength_ != 0; }
-    size_t computedSizeOfData() const {
-        return compressed() ? compressedLength_ : sizeof(jschar) * length_;
-    }
-    bool adjustDataSize(size_t nbytes);
-    const jschar *getOffThreadCompressionChars(ExclusiveContext *cx);
+    size_t computedSizeOfData() const;
 };
 
 class ScriptSourceHolder

js/src/jsworkers.cpp

@@ -894,8 +894,7 @@ WorkerThread::handleCompressionWorkload()
 
     {
         AutoUnlockWorkerThreadState unlock;
-        if (!compressionTask->work())
-            compressionTask->setOOM();
+        compressionTask->result = compressionTask->work();
     }
 
     compressionTask->workerThread = nullptr;
@@ -937,27 +936,36 @@ GlobalWorkerThreadState::compressionInProgress(SourceCompressionTask *task)
 bool
 SourceCompressionTask::complete()
 {
-    JS_ASSERT_IF(!ss, !chars);
-    if (active()) {
-        AutoLockWorkerThreadState lock;
+    if (!active()) {
+        JS_ASSERT(!compressed);
+        return true;
+    }
 
+    {
+        AutoLockWorkerThreadState lock;
         while (WorkerThreadState().compressionInProgress(this))
             WorkerThreadState().wait(GlobalWorkerThreadState::CONSUMER);
+    }
 
-        ss->ready_ = true;
+    if (result == Success) {
+        ss->setCompressedSource(compressed, compressedBytes);
 
         // Update memory accounting.
-        if (!oom)
-            cx->updateMallocCounter(ss->computedSizeOfData());
+        cx->updateMallocCounter(ss->computedSizeOfData());
+    } else {
+        js_free(compressed);
 
-        ss = nullptr;
-        chars = nullptr;
+        if (result == OOM)
+            js_ReportOutOfMemory(cx);
+        else if (result == Aborted && !ss->ensureOwnsSource(cx))
+            result = OOM;
     }
-    if (oom) {
-        js_ReportOutOfMemory(cx);
-        return false;
-    }
-    return true;
+
+    ss = nullptr;
+    compressed = nullptr;
+    JS_ASSERT(!active());
+
+    return result != OOM;
 }
 
 SourceCompressionTask *
@@ -977,30 +985,6 @@ GlobalWorkerThreadState::compressionTaskForSource(ScriptSource *ss)
     return nullptr;
 }
 
-const jschar *
-ScriptSource::getOffThreadCompressionChars(ExclusiveContext *cx)
-{
-    // If this is being compressed off thread, return its uncompressed chars.
-
-    if (ready()) {
-        // Compression has already finished on the source.
-        return nullptr;
-    }
-
-    AutoLockWorkerThreadState lock;
-
-    // Look for a token that hasn't finished compressing and whose source is
-    // the given ScriptSource.
-    if (SourceCompressionTask *task = WorkerThreadState().compressionTaskForSource(this))
-        return task->uncompressedChars();
-
-    // Compressing has finished, so this ScriptSource is ready. Avoid future
-    // queries on the worker thread state when getting the chars.
-    ready_ = true;
-
-    return nullptr;
-}
-
 void
 WorkerThread::threadLoop()
 {
@@ -1097,17 +1081,10 @@ js::StartOffThreadCompression(ExclusiveContext *cx, SourceCompressionTask *task)
 bool
 SourceCompressionTask::complete()
 {
-    JS_ASSERT(!active() && !oom);
+    JS_ASSERT(!ss);
     return true;
 }
 
-const jschar *
-ScriptSource::getOffThreadCompressionChars(ExclusiveContext *cx)
-{
-    JS_ASSERT(ready());
-    return nullptr;
-}
-
 frontend::CompileError &
 ExclusiveContext::addPendingCompileError()
 {

js/src/jsworkers.h

@@ -444,6 +444,7 @@ OffThreadParsingMustWaitForGC(JSRuntime *rt);
 struct SourceCompressionTask
 {
     friend class ScriptSource;
+    friend class WorkerThread;
 
 #ifdef JS_THREADSAFE
     // Thread performing the compression.
@@ -455,16 +456,24 @@ struct SourceCompressionTask
     ExclusiveContext *cx;
 
     ScriptSource *ss;
-    const jschar *chars;
-    bool oom;
 
     // Atomic flag to indicate to a worker thread that it should abort
     // compression on the source.
     mozilla::Atomic<bool, mozilla::Relaxed> abort_;
 
+    // Stores the result of the compression.
+    enum ResultType {
+        OOM,
+        Aborted,
+        Success
+    } result;
+    void *compressed;
+    size_t compressedBytes;
+
   public:
     explicit SourceCompressionTask(ExclusiveContext *cx)
-      : cx(cx), ss(nullptr), chars(nullptr), oom(false), abort_(false)
+      : cx(cx), ss(nullptr), abort_(false),
+        result(OOM), compressed(nullptr), compressedBytes(0)
     {
 #ifdef JS_THREADSAFE
         workerThread = nullptr;
@@ -476,13 +485,11 @@ struct SourceCompressionTask
         complete();
     }
 
-    bool work();
+    ResultType work();
     bool complete();
     void abort() { abort_ = true; }
     bool active() const { return !!ss; }
     ScriptSource *source() { return ss; }
-    const jschar *uncompressedChars() { return chars; }
-    void setOOM() { oom = true; }
 };
 
 } /* namespace js */