Bug 1593399 - Rework how mark colors are handled in weakmap marking r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51492

Bug 1594054 - Move ExecutableAllocator from JitRuntime to JitZone. r=jonco,erahm This matches the JitCode GC-thing lifetime and will hopefully help avoid fragmentation. Differential Revision: https://phabricator.services.mozilla.com/D52823

Bug 1593399 - Rework how mark colors are handled in weakmap marking r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51492

Bug 1593399 - Rework how mark colors are handled in weakmap marking r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51492

Bug 1592992 - Part 19: Move jsutil.cpp to util/Utility.cpp. r=jonco This file provides the implementation of js/Utility.h, so it should be renamed to match the header name. Differential Revision: https://phabricator.services.mozilla.com/D51378

Bug 1592992 - Part 18: Remove jsutil.h. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51377

Bug 1592992 - Part 17: Remove includes for jsutil.h. r=jonco And handle the fallout of missing #includes when compiling a non-unified build. Differential Revision: https://phabricator.services.mozilla.com/D51376

Bug 1592992 - Part 16: Include util/DiagnosticAssertions.h where necessary. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51375

Bug 1592992 - Part 15: Include util/Poison.h where necessary. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51374

Bug 1592992 - Part 14: Include util/Memory.h where necessary. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51373

Bug 1592992 - Part 13: Include util/BitArray.h where necessary. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51372

Bug 1592992 - Part 12: Replace js::Min/Max with std::min/max. r=jonco For the most part another mechanical change, except for two changes: - In gc/Nursery.cpp and vm/DateTime.cpp, the initializer_list variants for std::min/max are used, because the two parameter forms of min/max take const reference parameters. And it's not possible to take the reference of a constexpr variable which wasn't previously explicitly defined. This is just a C++14 limitation and as soon as we compile with C++17, we can revert this change. - builtin/String.cpp was calling `Max(unsignedVar, 0U)` in a few places, which led to compiler warnings, because the result is always `unsignedVar`. Differential Revision: https://phabricator.services.mozilla.com/D51371

Bug 1592992 - Part 11: Remove no longer needed includes from jsutil.h. r=jonco The includes are still transitively reachable through the new "#include util/*.h" headers. Differential Revision: https://phabricator.services.mozilla.com/D51370

Bug 1592992 - Part 10: Split diagnostic assertions from jsutil.h. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51369

Bug 1592992 - Part 9: Move js::IsInitialized() into its sole caller. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51368

Bug 1592992 - Part 8: Split memory and byte alignment functions from jsutil.h. r=jonco A single header for both memory and byte alignment functions, similar to the standard <memory> header, which also provides memory and byte alignment related functionality. Differential Revision: https://phabricator.services.mozilla.com/D51367

Bug 1592992 - Part 7: Split bit-array functions from jsutil.h. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51366

Bug 1592992 - Part 6: Split poisoning functions from jsutil.h. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51365

Bug 1592992 - Part 5: Replace js::Reverse with std::reverse. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51364

Bug 1592992 - Part 4: Replace js::EqualContainers with std::equal. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51363

Bug 1592992 - Part 3: Replace js::EraseIf with Vector::eraseIf where possible. r=jonco mozilla::Vector already provides an `eraseIf` method, so we should prefer to use the existing method. Except that Vector::eraseIf doesn't return the number of elements removed, but that's easy to determine manually. More annoyingly is the use of a modifying predicate function in Zone.cpp, which prevents using Vector::eraseIf. It looks like for that specific use case a custome `EraseIf` function is still necessary. Differential Revision: https://phabricator.services.mozilla.com/D51362

Bug 1592992 - Part 2: Replace js::Find with std::find. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51361

Bug 1592992 - Part 1: Remove unused jsutil functions. r=jonco AddContainerToHash and ForEach aren't used (anymore). Differential Revision: https://phabricator.services.mozilla.com/D51360

Bug 1593975 - update test. r=jonco check if enqueueMark is defined in opt build. Differential Revision: https://phabricator.services.mozilla.com/D51994

Bug 1593975 - update linearWeakMarkingDisabled_ to MainThreadOrGCTaskData. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51810

Bug 1593975 - update linearWeakMarkingDisabled_ to MainThreadOrGCTaskData. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51810

Bug 1587638: Add atom map to XDRIncrementalEncoder and atom table to XDRDecoder r=jonco,tcampbell The code that uses these fields is part of a subsequent patch. This patch is just getting the tracing right. Differential Revision: https://phabricator.services.mozilla.com/D48782

Bug 1531716 - Part 4: Replace jstypes macros with constexpr functions. r=jonco JS_BIT and JS_BITMASK are only used in contexts where uint32_t is used, so these two functions are now typed to accept and return uint32_t. JS_HOWMANY and the three JS_ROUND functions are only used with size_t inputs, so these four functions are now typed to accept and return size_t. Differential Revision: https://phabricator.services.mozilla.com/D51142

Bug 1531716 - Part 3: Replace ROUNDUP with JS_ROUNDUP. r=jonco Both macros compute the same result, so we can replace ROUNDUP with JS_ROUNDUP. Differential Revision: https://phabricator.services.mozilla.com/D51141

Bug 1531716 - Part 2: Remove unused macro in vm/Probes.cpp. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51140

Bug 1531716 - Part 1: Remove macros for generating the SmallChars array. r=jonco And the two macros to convert to and from SmallChars. Differential Revision: https://phabricator.services.mozilla.com/D51139

Bug 1591342: When setting breakpoints, require usable cross-compartment wrappers. r=jonco When the `Debugger` API sets a breakpoint in a JSScript or wasm::Instance, the BreakpointSite and Breakpoint objects belong to the code's compartment (logically, at least - they're C++ objects and don't actually have any compartment). Since a `Debugger` and its debuggees must be in separate compartments, the Breakpoint's references to its owning `Debugger` and its handler object must go through cross-compartment wrappers. If we have nuked the `Debugger`'s compartment, it's not clear how we're still trying to set breakpoints in its debuggees, but we should at least throw an error, to capture a JavaScript stack when it occurs. Differential Revision: https://phabricator.services.mozilla.com/D51210

Bug 1580888 - Split mark stack into main stack and spare stack. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D50689

Bug 1591342: When setting breakpoints, require usable cross-compartment wrappers. r=jonco When the `Debugger` API sets a breakpoint in a JSScript or wasm::Instance, the BreakpointSite and Breakpoint objects belong to the code's compartment (logically, at least - they're C++ objects and don't actually have any compartment). Since a `Debugger` and its debuggees must be in separate compartments, the Breakpoint's references to its owning `Debugger` and its handler object must go through cross-compartment wrappers. If we have nuked the `Debugger`'s compartment, it's not clear how we're still trying to set breakpoints in its debuggees, but we should at least throw an error, to capture a JavaScript stack when it occurs. Differential Revision: https://phabricator.services.mozilla.com/D51210

Bug 1585536: Track estimated malloc memory use for ICU objects. r=jonco The estimated memory is based on the maximum observed memory usage when running the Java script attached to the bug report. Differential Revision: https://phabricator.services.mozilla.com/D51087

Bug 1585536: Track estimated malloc memory use for ICU objects. r=jonco The estimated memory is based on the maximum observed memory usage when running the Java script attached to the bug report. Differential Revision: https://phabricator.services.mozilla.com/D51087

Bug 1580888 - Split mark stack into main stack and spare stack. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D50689

Bug 1580888 - Stop holding onto mark stack memory between GCs r=jonco Differential Revision: https://phabricator.services.mozilla.com/D50690

Bug 1592487 - update 'delayedMarkingWorkAdded' and 'markLaterArenas' to MainThreadOrGCTaskData. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D51091

Bug 1580888 - Split mark stack into main stack and spare stack. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D50689

Bug 1580888 - Stop holding onto mark stack memory between GCs r=jonco Differential Revision: https://phabricator.services.mozilla.com/D50690

Bug 1591889 - bail out early if task is already joined. r=jonco GCRuntime::joinTask will call recordParallelPhase(), and causes the parallel duration is multiplied when we call joinTask(sweepMarkTask) multiple times. Differential Revision: https://phabricator.services.mozilla.com/D50933

Bug 1584538 - Allow debug environment proxy to access aliased module variables after the module script executes, r=jonco. Differential Revision: https://phabricator.services.mozilla.com/D50747

Bug 1590845 - Add JS::MemoryUse::Embedding1 through 5. r=tcampbell,jonco This makes JS::AddAssociatedMemory() and JS::RemoveAssociatedMemory() more useful for embedders. Differential Revision: https://phabricator.services.mozilla.com/D50347

Bug 1586452: Let JSScripts and wasm::Instances own their BreakpointSites and Breakpoints. r=jonco Prior to this patch, the code marked a Debugger if any of its debuggee globals were marked, and it had breakpoints set in JSScripts/Instances that were marked. It cleaned up breakpoints by scanning all JSScripts and wasm::Instances in the zones being collected, looking for breakpoints set in them, and deleting them if either the JSScript/Instance or the owning Debugger was about to be finalized. That byzantine approach is equivalent to having owning edges from JSScripts/Instances to the breakpoints set in them, and from those breakpoints to the Debuggers they belong to. Proof: if a script is live, then its global must be live. If the script has a breakpoint set in it, and its global is live, then its Debugger will be marked, and hence neither the Debugger nor the script will be finalized, and hence the breakpoints will be retained. That second approach handles the situation with a much more traditional tracing process, wherein tracing a script traces its breakpoints, which traces their owning Debuggers. We can simply use cross-compartment wrappers for the edges from the breakpoints to their handlers and Debuggers, which removes a bunch of special cases for the GC to worry about. This patch changes things to use the second approach. - js::DebugScript and js::wasm::DebugState get `trace` and `finalize`/`delete_` methods, which get called from the right places. DebugScript and DebugState become the owners of their `BreakpointSite`s. - `BreakpointSite`s get `trace` and `finalize` methods that visit their `Breakpoint`s. `JSBreakpointSite`s also trace their scripts, and `WasmBreakpointSite`s trace their wasm::Instances. - `Breakpoint`s get a `trace` method that traces their owning `Debugger` and their handler object. They hold these as cross-compartment references, since the `Breakpoint` logically belongs to its code's compartment. Code that uses these fields wraps/unwraps as appropriate. - All the specialized breakpoint sweeping code is deleted: - `Zone::sweepBreakpoints` and its supporting `DebugAPI` functions are deleted. - `Debugger::hasAnyLiveHooks` no longer considers breakpoints. - `DebugAPI::markIteratively` no longer searches for breakpoints to mark. - `DebugAPI::traceAllForMovingGC` no longer traverses breakpoints. Differential Revision: https://phabricator.services.mozilla.com/D49858

Bug 1586452: Separate removal and destruction of Breakpoints into two methods. r=jonco Separate `Breakpoint::destroy` into two functions, `Breakpoint::remove`, which takes care of cleaning up empty sites, and `Breakpoint::delete_`, which only removes the breakpoint from its linked lists and frees its storage. Call `Breakpoint::remove` from sites that passed `True` for `destroy`'s `mayDestroySite` argument, and `Breakpoint::delete_` for those that passed `False`. The prior code had a `Breakpoint::destroy` method that took a `mayDestroySite` flag, which indicated whether the BreakpointSite should also be cleaned up, if the outgoing breakpoint was the last one left. Given that we never intend to leave empty BreakpointSites lying around, it was unclear why this option was needed. And indeed, it was passed as `True` (yes, please destroy the site if empty) from all call sites except one, in `js::wasm::DebugState::clearBreakpointsIn`. This function is used for both deleting individual breakpoints and removing all breakpoints set in a particular `wasm::Instance`, so it has two nested loops, the outer visiting all sites in the instance, and the inner visiting each site's breakpoints. Since `DebugState` stores `WasmBreakpointSites` in a hash table, allowing `Breakpoint::destroy` to remove empty `BreakpointSite`s would invalidate the outer loop's iteration, a classic bug. Instead, the inner loop forbade `Breakpoint::destroy` from removing the site, and cleaned it up itself, if empty, at the bottom of the outer loop body. Later patches in this series want a cleaner separation between the high-level operation of removing a single breakpoint, with whatever followup that entails, versus the low-level of operation of simply freeing the structure at hand. Differential Revision: https://phabricator.services.mozilla.com/D49857

Bug 1586452: SetBreakpointMatcher: success path should be main line. r=jonco No effect on behavior. It's SpiderMonkey's convention that the success path should be the top-level path through a function, and that failure should be handled by early returns. This patch makes SetBreakpointMatcher's methods conform to that convention. Differential Revision: https://phabricator.services.mozilla.com/D49856

Bug 1586452: Manage JIT traps when we create and remove breakpoint sites. r=jonco `Debugger` objects used to have an `enabled` property, which controlled (among other things) whether breakpoints would fire. This meant that it was possible to have a BreakpointSite that had breakpoints set in it, but yet should never trigger. Thus, whether or not the JIT code itself should be patched for the breakpoint depended on both 1) whether breakpoints were set, and 2) whether any of those breakpoints' owning Debuggers were enabled. To manage this, each BreakpointSite had not only a linked list of inserted breakpoints, but an 'enabled count' of how many of those breakpoints were in enabled Debuggers. Enabling/disabling a Debugger walked all its breakpoints and adjusted their sites' counts. When the enabled count was non-zero, the JIT code needed traps inserted; when it was zero, the trap sites could be filled with no-ops. The `enabled` property seemed like an innocent idea, but it turns out to have introduced all sorts of hair like this throughout the Debugger (for example, also making it harder for the GC to figure out what could be removed without observable effect), and then it turned out that the server never really wanted to toggle them on and off anyway. So we removed 'enabled' in bug 1564168. But this means that a BreakpointSite's enabled count should be non-zero if and only if its breakpoint list is non-empty; the count is redundant and can be removed. Further, since an empty BreakpointSite should be removed altogether, any inserted BreakpointSite can be assumed to have a non-zero enabled count. In other words, JIT code should have traps inserted if and only if a BreakpointSite is present for that code location. This means we can tie JIT code trap insertion and removal to BreakpointSite insertion and removal. Since BreakpointSite is an abstract base class, all the code that inserts and removes sites knows which concrete class it's creating, so we know statically how to go about patching the code. This means BreakpointSite no longer needs a 'recompile' pure virtual method. Its implementations get merged into the code that's inserting and removing BreakpointSite concrete instances. Differential Revision: https://phabricator.services.mozilla.com/D49855

Bug 1586452: Make `js::BreakpointSite::remove` the pure virtual method, and hoist `removeIfEmpty' into the base class. r=jonco The way a breakpoint site is removed depends on the concrete subclass, but deciding whether the site is empty or not is common to all BreakpointSites. There's no reason to repeat the condition in every implementation. Differential Revision: https://phabricator.services.mozilla.com/D49854

Bug 1586452: Remove class js::WasmBreakpoint; use plain js::Breakpoint instead. r=jonco All the information we need specific to Wasm breakpoints is available in the WasmBreakpointSite now, so there's no need for two different kinds of breakpoints any more. Code that used to obtain the wasm instance from the breakpoint now consults the site instead. BreakpointSite::allocSize confusingly provided the size of the site's *breakpoints*, not the site itself; but now it's no longer needed at all, and can be removed. Differential Revision: https://phabricator.services.mozilla.com/D49853