Bug 1453028 - Refactor the way we parse zeal mode strings r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Tue, 17 Apr 2018 08:44:55 +0200
changeset 414070 0f26c7b0aa7d9a4ef77bb4819e53c46d663e88b8
parent 414069 da8af21b243efa5c5810ee43d947f43865c238e5
child 414071 12337456bdd9cefdc8663d0c1fc1c1040247c616
push id33858
push userncsoregi@mozilla.com
push dateTue, 17 Apr 2018 21:55:44 +0000
treeherdermozilla-central@d6eb5597d744 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1453028
milestone61.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 1453028 - Refactor the way we parse zeal mode strings r=sfink
js/src/gc/GC.cpp
js/src/gc/GCEnum.h
js/src/gc/GCRuntime.h
js/src/shell/js.cpp
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -187,16 +187,17 @@
 
 #include "gc/GC-inl.h"
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/MacroForEach.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Move.h"
+#include "mozilla/Range.h"
 #include "mozilla/ScopeExit.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/TypeTraits.h"
 #include "mozilla/Unused.h"
 
 #include <ctype.h>
 #include <initializer_list>
 #include <string.h>
@@ -1084,76 +1085,118 @@ GCRuntime::setZeal(uint8_t zeal, uint32_
 }
 
 void
 GCRuntime::setNextScheduled(uint32_t count)
 {
     nextScheduled = count;
 }
 
-bool
-GCRuntime::parseAndSetZeal(const char* str)
-{
-    int frequency = -1;
-    bool foundFrequency = false;
-    mozilla::Vector<int, 0, SystemAllocPolicy> zeals;
-
-    static const struct {
-        const char* const zealMode;
+using CharRange = mozilla::Range<const char>;
+using CharRangeVector = Vector<CharRange, 0, SystemAllocPolicy>;
+
+static bool
+ParseZealModeName(CharRange text, uint32_t* modeOut)
+{
+    struct ModeInfo
+    {
+        const char* name;
         size_t length;
-        uint32_t zeal;
-    } zealModes[] = {
-#define ZEAL_MODE(name, value) {#name, sizeof(#name) - 1, value},
+        uint32_t value;
+    };
+
+    static const ModeInfo zealModes[] = {
+        {"None", 0},
+#define ZEAL_MODE(name, value) {#name, strlen(#name), value},
         JS_FOR_EACH_ZEAL_MODE(ZEAL_MODE)
 #undef ZEAL_MODE
-        {"None", 4, 0}
     };
 
-    do {
-        int zeal = -1;
-
-        const char* p = nullptr;
-        if (isdigit(str[0])) {
-            zeal = atoi(str);
-
-            size_t offset = strspn(str, "0123456789");
-            p = str + offset;
-        } else {
-            for (auto z : zealModes) {
-                if (!strncmp(str, z.zealMode, z.length)) {
-                    zeal = z.zeal;
-                    p = str + z.length;
-                    break;
-                }
-            }
-        }
-        if (p) {
-            if (!*p || *p == ';') {
-                frequency = JS_DEFAULT_ZEAL_FREQ;
-            } else if (*p == ',') {
-                frequency = atoi(p + 1);
-                foundFrequency = true;
-            }
-        }
-
-        if (zeal < 0 || zeal > int(ZealMode::Limit) || frequency <= 0) {
-            fprintf(stderr, "Format: JS_GC_ZEAL=level(;level)*[,N]\n");
-            fputs(ZealModeHelpText, stderr);
+    for (auto mode : zealModes) {
+        if (text.length() == mode.length &&
+            memcmp(text.begin().get(), mode.name, mode.length) == 0)
+        {
+            *modeOut = mode.value;
+            return true;
+        }
+    }
+
+    return false;
+}
+
+static bool
+ParseZealModeNumericParam(CharRange text, uint32_t* paramOut)
+{
+    if (text.length() == 0)
+        return false;
+
+    for (auto c : text) {
+        if (!isdigit(c))
             return false;
-        }
-
-        if (!zeals.emplaceBack(zeal)) {
-            return false;
-        }
-    } while (!foundFrequency &&
-             (str = strchr(str, ';')) != nullptr &&
-             str++);
-
-    for (auto z : zeals)
-        setZeal(z, frequency);
+    }
+
+    *paramOut = atoi(text.begin().get());
+    return true;
+}
+
+static bool
+SplitStringBy(CharRange text, char delimiter, CharRangeVector* result)
+{
+    auto start = text.begin();
+    for (auto ptr = start; ptr != text.end(); ptr++) {
+        if (*ptr == delimiter) {
+            if (!result->emplaceBack(start, ptr))
+                return false;
+            start = ptr + 1;
+        }
+    }
+
+    return result->emplaceBack(start, text.end());
+}
+
+static bool
+PrintZealHelpAndFail()
+{
+    fprintf(stderr, "Format: JS_GC_ZEAL=level(;level)*[,N]\n");
+    fputs(ZealModeHelpText, stderr);
+    return false;
+}
+
+bool
+GCRuntime::parseAndSetZeal(const char* str)
+{
+    // Set the zeal mode from a string consisting of one or more mode specifiers
+    // separated by ';', optionally followed by a ',' and the trigger frequency.
+    // The mode specifiers can by a mode name or its number.
+
+    auto text = CharRange(str, strlen(str));
+
+    CharRangeVector parts;
+    if (!SplitStringBy(text, ',', &parts))
+        return false;
+
+    if (parts.length() == 0 || parts.length() > 2)
+        return PrintZealHelpAndFail();
+
+    uint32_t frequency = JS_DEFAULT_ZEAL_FREQ;
+    if (parts.length() == 2 && !ParseZealModeNumericParam(parts[1], &frequency))
+        return PrintZealHelpAndFail();
+
+    CharRangeVector modes;
+    if (!SplitStringBy(parts[0], ';', &modes))
+        return false;
+
+    for (const auto& descr : modes) {
+        uint32_t mode;
+        if (!ParseZealModeName(descr, &mode) && !ParseZealModeNumericParam(descr, &mode))
+            return PrintZealHelpAndFail();
+
+        setZeal(mode, frequency);
+    }
+
     return true;
 }
 
 static const char*
 AllocKindName(AllocKind kind)
 {
     static const char* names[] = {
 #define EXPAND_THING_NAME(allocKind, _1, _2, _3, _4, _5) \
--- a/js/src/gc/GCEnum.h
+++ b/js/src/gc/GCEnum.h
@@ -73,15 +73,16 @@ enum class AbortReason {
     D(CheckNursery, 16)                \
     D(IncrementalSweepThenFinish, 17)  \
     D(CheckGrayMarking, 18)
 
 enum class ZealMode {
 #define ZEAL_MODE(name, value) name = value,
     JS_FOR_EACH_ZEAL_MODE(ZEAL_MODE)
 #undef ZEAL_MODE
-    Limit = 18
+    Count,
+    Limit = Count - 1
 };
 
 } /* namespace gc */
 } /* namespace js */
 
 #endif /* gc_GCEnum_h */
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -889,16 +889,18 @@ class GCRuntime
      * in gc/Verifier.cpp for more information about this.
      *
      * zeal_ values from 8 to 10 periodically run different types of
      * incremental GC.
      *
      * zeal_ value 14 performs periodic shrinking collections.
      */
 #ifdef JS_GC_ZEAL
+    static_assert(size_t(ZealMode::Count) <= 32,
+                  "Too many zeal modes to store in a uint32_t");
     ActiveThreadData<uint32_t> zealModeBits;
     ActiveThreadData<int> zealFrequency;
     ActiveThreadData<int> nextScheduled;
     ActiveThreadData<bool> deterministicOnly;
     ActiveThreadData<int> incrementalLimit;
 
     ActiveThreadData<Vector<JSObject*, 0, SystemAllocPolicy>> selectedForMarking;
 #endif
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -8622,21 +8622,20 @@ SetWorkerContextOptions(JSContext* cx)
                              .setNativeRegExp(enableNativeRegExp)
                              .setStreams(enableStreams)
                              .setArrayProtoValues(enableArrayProtoValues);
     cx->runtime()->setOffthreadIonCompilationEnabled(offthreadCompilation);
     cx->runtime()->profilingScripts = enableCodeCoverage || enableDisassemblyDumps;
 
 #ifdef JS_GC_ZEAL
     if (gZealBits && gZealFrequency) {
-#define ZEAL_MODE(_, value)                        \
-        if (gZealBits & (1 << value))              \
-            cx->runtime()->gc.setZeal(value, gZealFrequency);
-        JS_FOR_EACH_ZEAL_MODE(ZEAL_MODE)
-#undef ZEAL_MODE
+        for (size_t i = 0; i < size_t(gc::ZealMode::Count); i++) {
+            if (gZealBits & (1 << i))
+                cx->runtime()->gc.setZeal(i, gZealFrequency);
+        }
     }
 #endif
 
     JS_SetNativeStackQuota(cx, gMaxStackSize);
 }
 
 static int
 Shell(JSContext* cx, OptionParser* op, char** envp)