Bug 1250935 - Use mozilla::Variant instead of a raw union and manual tag for ScriptSource::data; r=terrence
authorNick Fitzgerald <fitzgen@gmail.com>
Wed, 24 Feb 2016 09:46:00 +0100
changeset 321883 e799f20cf69b5a203a1b8e44fa284b243f38a8a4
parent 321882 66a7d72ae7705dc26b0cfcf251618ae887f5d0c7
child 321884 b38ae7ec0fe23a94f1df75c34f56a38ff07b3789
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs1250935
milestone47.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1250935 - Use mozilla::Variant instead of a raw union and manual tag for ScriptSource::data; r=terrence By using mozilla::Variant, we can only access the underlying data in a type-safe manner, and are forced to handle all cases when matching on the variant type.
js/src/jsscript.cpp
js/src/jsscript.h
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -1925,55 +1925,76 @@ UncompressedSourceCache::sizeOfExcluding
         }
     }
     return n;
 }
 
 const char16_t*
 ScriptSource::chars(JSContext* cx, UncompressedSourceCache::AutoHoldEntry& holder)
 {
-    switch (dataType) {
-      case DataUncompressed:
-        return uncompressedChars();
-
-      case DataCompressed: {
-        if (const char16_t* decompressed = cx->runtime()->uncompressedSourceCache.lookup(this, holder))
+    struct CharsMatcher
+    {
+        using ReturnType = const char16_t*;
+
+        JSContext* cx;
+        ScriptSource& ss;
+        UncompressedSourceCache::AutoHoldEntry& holder;
+
+        explicit CharsMatcher(JSContext* cx, ScriptSource& ss,
+                              UncompressedSourceCache::AutoHoldEntry& holder)
+          : cx(cx)
+          , ss(ss)
+          , holder(holder)
+        { }
+
+        ReturnType match(Uncompressed& u) {
+            return u.chars;
+        }
+
+        ReturnType match(Compressed& c) {
+            if (const char16_t* decompressed = cx->runtime()->uncompressedSourceCache.lookup(&ss, holder))
+                return decompressed;
+
+            const size_t nbytes = sizeof(char16_t) * (ss.length() + 1);
+            char16_t* decompressed = static_cast<char16_t*>(js_malloc(nbytes));
+            if (!decompressed) {
+                JS_ReportOutOfMemory(cx);
+                return nullptr;
+            }
+
+            if (!DecompressString((const unsigned char*) ss.compressedData(), ss.compressedBytes(),
+                                  reinterpret_cast<unsigned char*>(decompressed), nbytes)) {
+                JS_ReportOutOfMemory(cx);
+                js_free(decompressed);
+                return nullptr;
+            }
+
+            decompressed[ss.length()] = 0;
+
+            if (!cx->runtime()->uncompressedSourceCache.put(&ss, decompressed, holder)) {
+                JS_ReportOutOfMemory(cx);
+                js_free(decompressed);
+                return nullptr;
+            }
+
             return decompressed;
-
-        const size_t nbytes = sizeof(char16_t) * (length_ + 1);
-        char16_t* decompressed = static_cast<char16_t*>(js_malloc(nbytes));
-        if (!decompressed) {
-            JS_ReportOutOfMemory(cx);
+        }
+
+        ReturnType match(Parent& p) {
+            return p.parent->chars(cx, holder);
+        }
+
+        ReturnType match(Missing&) {
+            MOZ_CRASH("ScriptSource::chars() on ScriptSource with SourceType = Missing");
             return nullptr;
         }
-
-        if (!DecompressString((const unsigned char*) compressedData(), compressedBytes(),
-                              reinterpret_cast<unsigned char*>(decompressed), nbytes)) {
-            JS_ReportOutOfMemory(cx);
-            js_free(decompressed);
-            return nullptr;
-        }
-
-        decompressed[length_] = 0;
-
-        if (!cx->runtime()->uncompressedSourceCache.put(this, decompressed, holder)) {
-            JS_ReportOutOfMemory(cx);
-            js_free(decompressed);
-            return nullptr;
-        }
-
-        return decompressed;
-      }
-
-      case DataParent:
-        return parent()->chars(cx, holder);
-
-      default:
-        MOZ_CRASH();
-    }
+    };
+
+    CharsMatcher cm(cx, *this, holder);
+    return data.match(cm);
 }
 
 JSFlatString*
 ScriptSource::substring(JSContext* cx, uint32_t start, uint32_t stop)
 {
     MOZ_ASSERT(start <= stop);
     UncompressedSourceCache::AutoHoldEntry holder;
     const char16_t* chars = this->chars(cx, holder);
@@ -1991,80 +2012,74 @@ ScriptSource::substringDontDeflate(JSCon
     if (!chars)
         return nullptr;
     return NewStringCopyNDontDeflate<CanGC>(cx, chars + start, stop - start);
 }
 
 void
 ScriptSource::setSource(const char16_t* chars, size_t length, bool ownsChars /* = true */)
 {
-    MOZ_ASSERT(dataType == DataMissing);
-
-    dataType = DataUncompressed;
-    data.uncompressed.chars = chars;
-    data.uncompressed.ownsChars = ownsChars;
-
+    MOZ_ASSERT(data.is<Missing>());
+
+    data = SourceType(Uncompressed(chars, ownsChars));
     length_ = length;
 }
 
 void
 ScriptSource::setCompressedSource(JSRuntime* maybert, void* raw, size_t nbytes, HashNumber hash)
 {
-    MOZ_ASSERT(dataType == DataMissing || dataType == DataUncompressed);
-    if (dataType == DataUncompressed && ownsUncompressedChars())
+    MOZ_ASSERT(data.is<Missing>() || data.is<Uncompressed>());
+
+    if (data.is<Uncompressed>() && data.as<Uncompressed>().ownsChars)
         js_free(const_cast<char16_t*>(uncompressedChars()));
 
-    dataType = DataCompressed;
-    data.compressed.raw = raw;
-    data.compressed.nbytes = nbytes;
-    data.compressed.hash = hash;
+    data = SourceType(Compressed(raw, nbytes, hash));
 
     if (maybert)
         updateCompressedSourceSet(maybert);
 }
 
 void
 ScriptSource::updateCompressedSourceSet(JSRuntime* rt)
 {
-    MOZ_ASSERT(dataType == DataCompressed);
+    MOZ_ASSERT(data.is<Compressed>());
     MOZ_ASSERT(!inCompressedSourceSet);
 
     CompressedSourceSet::AddPtr p = rt->compressedSourceSet.lookupForAdd(this);
     if (p) {
         // There is another ScriptSource with the same compressed data.
         // Mark that ScriptSource as the parent and use it for all attempts to
         // get the source for this ScriptSource.
         ScriptSource* parent = *p;
         parent->incref();
 
         js_free(compressedData());
-        dataType = DataParent;
-        data.parent = parent;
+        data = SourceType(Parent(parent));
     } else {
         if (rt->compressedSourceSet.add(p, this))
             inCompressedSourceSet = true;
     }
 }
 
 bool
 ScriptSource::ensureOwnsSource(ExclusiveContext* cx)
 {
-    MOZ_ASSERT(dataType == DataUncompressed);
+    MOZ_ASSERT(data.is<Uncompressed>());
     if (ownsUncompressedChars())
         return true;
 
     char16_t* uncompressed = cx->zone()->pod_malloc<char16_t>(Max<size_t>(length_, 1));
     if (!uncompressed) {
         ReportOutOfMemory(cx);
         return false;
     }
     PodCopy(uncompressed, uncompressedChars(), length_);
 
-    data.uncompressed.chars = uncompressed;
-    data.uncompressed.ownsChars = true;
+    data.as<Uncompressed>().chars = uncompressed;
+    data.as<Uncompressed>().ownsChars = true;
     return true;
 }
 
 bool
 ScriptSource::setSourceCopy(ExclusiveContext* cx, SourceBufferHolder& srcBuf,
                             bool argumentsNotIncluded, SourceCompressionTask* task)
 {
     MOZ_ASSERT(!hasSourceData());
@@ -2164,91 +2179,131 @@ SourceCompressionTask::work()
     if (void* newCompressed = js_realloc(compressed, compressedBytes))
         compressed = newCompressed;
 
     return Success;
 }
 
 ScriptSource::~ScriptSource()
 {
-    MOZ_ASSERT_IF(inCompressedSourceSet, dataType == DataCompressed);
-
-    switch (dataType) {
-      case DataUncompressed:
-        if (ownsUncompressedChars())
-            js_free(const_cast<char16_t*>(uncompressedChars()));
-        break;
-
-      case DataCompressed:
-        // Script source references are only manipulated on the main thread,
-        // except during off thread parsing when the source may be created
-        // and used exclusively by the thread doing the parse. In this case the
-        // ScriptSource might be destroyed while off the main thread, but it
-        // will not have been added to the runtime's compressed source set
-        // until the parse is finished on the main thread.
-        if (inCompressedSourceSet)
-            TlsPerThreadData.get()->runtimeFromMainThread()->compressedSourceSet.remove(this);
-        js_free(compressedData());
-        break;
-
-      case DataParent:
-        parent()->decref();
-        break;
-
-      default:
-        break;
-    }
+    struct DestroyMatcher
+    {
+        using ReturnType = void;
+
+        ScriptSource& ss;
+
+        explicit DestroyMatcher(ScriptSource& ss)
+          : ss(ss)
+        { }
+
+        ReturnType match(Uncompressed& u) {
+            if (u.ownsChars)
+                js_free(const_cast<char16_t*>(u.chars));
+        }
+
+        ReturnType match(Compressed& c) {
+            if (ss.inCompressedSourceSet)
+                TlsPerThreadData.get()->runtimeFromMainThread()->compressedSourceSet.remove(&ss);
+            js_free(c.raw);
+        }
+
+        ReturnType match(Parent& p) {
+            p.parent->decref();
+        }
+
+        ReturnType match(Missing&) {
+            // Nothing to do here.
+        }
+    };
+
+    MOZ_ASSERT_IF(inCompressedSourceSet, data.is<Compressed>());
+
+    DestroyMatcher dm(*this);
+    data.match(dm);
 }
 
 void
 ScriptSource::addSizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf,
                                      JS::ScriptSourceInfo* info) const
 {
-    if (dataType == DataUncompressed && ownsUncompressedChars())
+    if (data.is<Uncompressed>() && ownsUncompressedChars())
         info->uncompressed += mallocSizeOf(uncompressedChars());
-    else if (dataType == DataCompressed)
+    else if (data.is<Compressed>())
         info->compressed += mallocSizeOf(compressedData());
     info->misc += mallocSizeOf(this) +
                   mallocSizeOf(filename_.get()) +
                   mallocSizeOf(introducerFilename_.get());
     info->numScripts++;
 }
 
 template<XDRMode mode>
 bool
 ScriptSource::performXDR(XDRState<mode>* xdr)
 {
+    struct CompressedLengthMatcher
+    {
+        using ReturnType = size_t;
+
+        ReturnType match(Uncompressed&) {
+            return 0;
+        }
+
+        ReturnType match(Compressed& c) {
+            return c.nbytes;
+        }
+
+        ReturnType match(Parent& p) {
+            return p.parent->data.match(*this);
+        }
+
+        ReturnType match(Missing&) {
+            MOZ_CRASH("Missing source data in ScriptSource::performXDR");
+            return 0;
+        }
+    };
+
+    struct RawDataMatcher
+    {
+        using ReturnType = void*;
+
+        ReturnType match(Uncompressed& u) {
+            return (void*) u.chars;
+        }
+
+        ReturnType match(Compressed& c) {
+            return c.raw;
+        }
+
+        ReturnType match(Parent& p) {
+            return p.parent->data.match(*this);
+        }
+
+        ReturnType match(Missing&) {
+            MOZ_CRASH("Missing source data in ScriptSource::performXDR");
+            return nullptr;
+        }
+    };
+
     uint8_t hasSource = hasSourceData();
     if (!xdr->codeUint8(&hasSource))
         return false;
 
     uint8_t retrievable = sourceRetrievable_;
     if (!xdr->codeUint8(&retrievable))
         return false;
     sourceRetrievable_ = retrievable;
 
     if (hasSource && !sourceRetrievable_) {
         if (!xdr->codeUint32(&length_))
             return false;
 
         uint32_t compressedLength;
         if (mode == XDR_ENCODE) {
-            switch (dataType) {
-              case DataUncompressed:
-                compressedLength = 0;
-                break;
-              case DataCompressed:
-                compressedLength = compressedBytes();
-                break;
-              case DataParent:
-                compressedLength = parent()->compressedBytes();
-                break;
-              default:
-                MOZ_CRASH();
-            }
+            CompressedLengthMatcher m;
+            compressedLength = data.match(m);
         }
         if (!xdr->codeUint32(&compressedLength))
             return false;
 
         {
             uint8_t argumentsNotIncluded;
             if (mode == XDR_ENCODE)
                 argumentsNotIncluded = argumentsNotIncluded_;
@@ -2267,30 +2322,18 @@ ScriptSource::performXDR(XDRState<mode>*
             }
 
             if (compressedLength)
                 setCompressedSource(xdr->cx()->runtime(), p, compressedLength,
                                     CompressedSourceHasher::computeHash(p, compressedLength));
             else
                 setSource((const char16_t*) p, length_);
         } else {
-            void* p;
-            switch (dataType) {
-              case DataUncompressed:
-                p = (void*) uncompressedChars();
-                break;
-              case DataCompressed:
-                p = compressedData();
-                break;
-              case DataParent:
-                p = parent()->compressedData();
-                break;
-              default:
-                MOZ_CRASH();
-            }
+            RawDataMatcher rdm;
+            void* p = data.match(rdm);
             if (!xdr->codeBytes(p, byteLen))
                 return false;
         }
     }
 
     uint8_t haveSourceMap = hasSourceMapURL();
     if (!xdr->codeUint8(&haveSourceMap))
         return false;
@@ -2461,19 +2504,19 @@ ScriptSource::setSourceMapURL(ExclusiveC
 
     sourceMapURL_ = DuplicateString(cx, sourceMapURL);
     return sourceMapURL_ != nullptr;
 }
 
 size_t
 ScriptSource::computedSizeOfData() const
 {
-    if (dataType == DataUncompressed && ownsUncompressedChars())
+    if (data.is<Uncompressed>() && ownsUncompressedChars())
         return sizeof(char16_t) * length_;
-    if (dataType == DataCompressed)
+    if (data.is<Compressed>())
         return compressedBytes();
     return 0;
 }
 
 /*
  * Shared script data management.
  */
 
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -7,16 +7,17 @@
 /* JS script descriptor. */
 
 #ifndef jsscript_h
 #define jsscript_h
 
 #include "mozilla/Atomics.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/PodOperations.h"
+#include "mozilla/Variant.h"
 
 #include "jsatom.h"
 #include "jslock.h"
 #include "jsopcode.h"
 #include "jstypes.h"
 
 #include "gc/Barrier.h"
 #include "gc/Rooting.h"
@@ -601,37 +602,54 @@ class ScriptSource
 
     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,
-        DataParent
-    } dataType;
-
-    union {
-        struct {
-            const char16_t* chars;
-            bool ownsChars;
-        } uncompressed;
-
-        struct {
-            void* raw;
-            size_t nbytes;
-            HashNumber hash;
-        } compressed;
+
+    struct Missing { };
+
+    struct Uncompressed
+    {
+        Uncompressed(const char16_t* chars, bool ownsChars)
+          : chars(chars)
+          , ownsChars(ownsChars)
+        { }
+
+        const char16_t* chars;
+        bool ownsChars;
+    };
+
+    struct Compressed
+    {
+        Compressed(void* raw, size_t nbytes, HashNumber hash)
+          : raw(raw)
+          , nbytes(nbytes)
+          , hash(hash)
+        { }
+
+        void* raw;
+        size_t nbytes;
+        HashNumber hash;
+    };
+
+    struct Parent
+    {
+        explicit Parent(ScriptSource* parent)
+          : parent(parent)
+        { }
 
         ScriptSource* parent;
-    } data;
+    };
+
+    using SourceType = mozilla::Variant<Missing, Uncompressed, Compressed, Parent>;
+    SourceType data;
 
     uint32_t length_;
 
     // The filename of this script.
     UniqueChars filename_;
 
     UniqueTwoByteChars displayURL_;
     UniqueTwoByteChars sourceMapURL_;
@@ -675,17 +693,17 @@ class ScriptSource
     bool hasIntroductionOffset_:1;
 
     // Whether this is in the runtime's set of compressed ScriptSources.
     bool inCompressedSourceSet:1;
 
   public:
     explicit ScriptSource()
       : refs(0),
-        dataType(DataMissing),
+        data(SourceType(Missing())),
         length_(0),
         filename_(nullptr),
         displayURL_(nullptr),
         sourceMapURL_(nullptr),
         mutedErrors_(false),
         introductionOffset_(0),
         introducerFilename_(nullptr),
         introductionType_(nullptr),
@@ -704,60 +722,54 @@ class ScriptSource
     }
     bool initFromOptions(ExclusiveContext* cx, const ReadOnlyCompileOptions& options);
     bool setSourceCopy(ExclusiveContext* cx,
                        JS::SourceBufferHolder& srcBuf,
                        bool argumentsNotIncluded,
                        SourceCompressionTask* tok);
     void setSourceRetrievable() { sourceRetrievable_ = true; }
     bool sourceRetrievable() const { return sourceRetrievable_; }
-    bool hasSourceData() const { return dataType != DataMissing; }
-    bool hasCompressedSource() const { return dataType == DataCompressed; }
+    bool hasSourceData() const { return !data.is<Missing>(); }
+    bool hasCompressedSource() const { return data.is<Compressed>(); }
     size_t length() const {
         MOZ_ASSERT(hasSourceData());
         return length_;
     }
     bool argumentsNotIncluded() const {
         MOZ_ASSERT(hasSourceData());
         return argumentsNotIncluded_;
     }
     const char16_t* chars(JSContext* cx, UncompressedSourceCache::AutoHoldEntry& asp);
     JSFlatString* substring(JSContext* cx, uint32_t start, uint32_t stop);
     JSFlatString* substringDontDeflate(JSContext* cx, uint32_t start, uint32_t stop);
     void addSizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf,
                                 JS::ScriptSourceInfo* info) const;
 
     const char16_t* uncompressedChars() const {
-        MOZ_ASSERT(dataType == DataUncompressed);
-        return data.uncompressed.chars;
+        return data.as<Uncompressed>().chars;
     }
 
     bool ownsUncompressedChars() const {
-        MOZ_ASSERT(dataType == DataUncompressed);
-        return data.uncompressed.ownsChars;
+        return data.as<Uncompressed>().ownsChars;
     }
 
     void* compressedData() const {
-        MOZ_ASSERT(dataType == DataCompressed);
-        return data.compressed.raw;
+        return data.as<Compressed>().raw;
     }
 
     size_t compressedBytes() const {
-        MOZ_ASSERT(dataType == DataCompressed);
-        return data.compressed.nbytes;
+        return data.as<Compressed>().nbytes;
     }
 
     HashNumber compressedHash() const {
-        MOZ_ASSERT(dataType == DataCompressed);
-        return data.compressed.hash;
+        return data.as<Compressed>().hash;
     }
 
     ScriptSource* parent() const {
-        MOZ_ASSERT(dataType == DataParent);
-        return data.parent;
+        return data.as<Parent>().parent;
     }
 
     void setSource(const char16_t* chars, size_t length, bool ownsChars = true);
     void setCompressedSource(JSRuntime* maybert, void* raw, size_t nbytes, HashNumber hash);
     void updateCompressedSourceSet(JSRuntime* rt);
     bool ensureOwnsSource(ExclusiveContext* cx);
 
     // XDR handling