Closed Bug 1643650 Opened 5 years ago Closed 5 years ago

Refactor nursery collection

Categories

(Core :: JavaScript: GC, task, P3)

Product:
Core
Component:
JavaScript: GC
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla79
Tracking Flags:
Tracking Status
firefox79 --- fixed

People

(Reporter: jonco, Assigned: jonco)

References

Details

Attachments

(5 files)

Bug 1643650 - Refactor nursery to update previous collection data in one place r?sfink
47 bytes, text/x-phabricator-request
Details | Review
Bug 1643650 - Refactor nursery to move all telemetry to one method r?sfink
47 bytes, text/x-phabricator-request
Details | Review
Bug 1643650 - Refactor nursery to use double for floating point variable everywhere r?sfink
47 bytes, text/x-phabricator-request
Details | Review
Bug 1643650 - Round nursery size parameters once when they are set rather than on every use r?sfink
47 bytes, text/x-phabricator-request
Details | Review
Bug 1643650 - Refactor nursery resizing r?sfink
47 bytes, text/x-phabricator-request
Details | Review

Nursery collection has become pretty complicated and there's a few things we can do to simplify it.

This also splits out profiling output from the main collect() method.

Depends on D78494

This caused problems elsewhere due to limited resolution of 'float'. double is the default type for floating point values in C++ and we should use it everywhere unless there is a compelling reason not.

Depends on D78495

This has the side effect that gcparam() will report the values that are actually in use rather than the value that are set, which seems like a good thing. I had to update test code expectations due to this.

Depends on D78497

Nursery reszing has got pretty complicted. This simplifies things by factoring out the ideal size calculation and then clamping the result to the allowed size range.

Depends on D78500

Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fd4c3045b895 Refactor nursery to update previous collection data in one place r=sfink https://hg.mozilla.org/integration/autoland/rev/d4c40dbb2488 Refactor nursery to move all telemetry to one method r=sfink https://hg.mozilla.org/integration/autoland/rev/f2e3001a9076 Refactor nursery to use double for floating point variable everywhere r=sfink https://hg.mozilla.org/integration/autoland/rev/76a609e569bf Round nursery size parameters once when they are set rather than on every use r=sfink https://hg.mozilla.org/integration/autoland/rev/596abb4cb54d Refactor nursery resizing r=sfink

Backed out 5 changesets (bug 1643650) for Nursery.cpp related bustage.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=596abb4cb54d75bfbfb9d7eef039cb25161e27b4&searchStr=linux%2Cx64%2Cdebug%2Cbuild-linux64-plain%2Fdebug%2C%28bp%29&tochange=2213fde055b8b1eecff4e69743589554f59e1a04&selectedTaskRun=RYtj22QeRTu50kTU_xLXug-0

Backout link: https://hg.mozilla.org/integration/autoland/rev/2213fde055b8b1eecff4e69743589554f59e1a04

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=305467730&repo=autoland&lineNumber=6514

[task 2020-06-08T14:01:35.312Z] 14:01:35     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/js/src/gc'
[task 2020-06-08T14:01:35.313Z] 14:01:35     INFO -  /builds/worker/fetches/clang/bin/clang++ -std=gnu++17 -o Unified_cpp_js_src_gc1.o -c  -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -ftrivial-auto-var-init=pattern -DDEBUG=1 -DWASM_SUPPORTS_HUGE_MEMORY -DJS_CACHEIR_SPEW -DJS_STRUCTURED_SPEW -DJS_HAS_CTYPES -DFFI_BUILDING -DEXPORT_JS_API -DMOZ_HAS_MOZGLUE -I/builds/worker/checkouts/gecko/js/src/gc -I/builds/worker/workspace/obj-build/js/src/gc -I/builds/worker/workspace/obj-build/js/src -I/builds/worker/checkouts/gecko/js/src -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/js/src/js-confdefs.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-noexcept-type -Wno-unknown-warning-option -fno-sized-deallocation -fno-aligned-new -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -O3 -fno-omit-frame-pointer -funwind-tables -Werror -fno-strict-aliasing -Werror=format -Wno-shadow -fexperimental-new-pass-manager  -MD -MP -MF .deps/Unified_cpp_js_src_gc1.o.pp   Unified_cpp_js_src_gc1.cpp
[task 2020-06-08T14:01:35.314Z] 14:01:35     INFO -  In file included from Unified_cpp_js_src_gc1.cpp:20:
[task 2020-06-08T14:01:35.314Z] 14:01:35    ERROR -  /builds/worker/checkouts/gecko/js/src/gc/Nursery.cpp:1492:15: error: use of undeclared identifier 'isnan'
[task 2020-06-08T14:01:35.314Z] 14:01:35     INFO -    MOZ_ASSERT(!isnan(value) && !isnan(min) && !isnan(max));
[task 2020-06-08T14:01:35.314Z] 14:01:35     INFO -                ^
[task 2020-06-08T14:01:35.314Z] 14:01:35    ERROR -  /builds/worker/checkouts/gecko/js/src/gc/Nursery.cpp:1492:32: error: use of undeclared identifier 'isnan'
[task 2020-06-08T14:01:35.314Z] 14:01:35     INFO -    MOZ_ASSERT(!isnan(value) && !isnan(min) && !isnan(max));
[task 2020-06-08T14:01:35.314Z] 14:01:35     INFO -                                 ^
[task 2020-06-08T14:01:35.314Z] 14:01:35    ERROR -  /builds/worker/checkouts/gecko/js/src/gc/Nursery.cpp:1492:47: error: use of undeclared identifier 'isnan'
[task 2020-06-08T14:01:35.314Z] 14:01:35     INFO -    MOZ_ASSERT(!isnan(value) && !isnan(min) && !isnan(max));
[task 2020-06-08T14:01:35.314Z] 14:01:35     INFO -                                                ^
[task 2020-06-08T14:01:35.314Z] 14:01:35    ERROR -  /builds/worker/checkouts/gecko/js/src/gc/Nursery.cpp:1492:15: error: use of undeclared identifier 'isnan'
[task 2020-06-08T14:01:35.314Z] 14:01:35     INFO -    MOZ_ASSERT(!isnan(value) && !isnan(min) && !isnan(max));
[task 2020-06-08T14:01:35.315Z] 14:01:35     INFO -                ^
[task 2020-06-08T14:01:35.315Z] 14:01:35    ERROR -  /builds/worker/checkouts/gecko/js/src/gc/Nursery.cpp:1492:32: error: use of undeclared identifier 'isnan'
[task 2020-06-08T14:01:35.315Z] 14:01:35     INFO -    MOZ_ASSERT(!isnan(value) && !isnan(min) && !isnan(max));
[task 2020-06-08T14:01:35.315Z] 14:01:35     INFO -                                 ^
[task 2020-06-08T14:01:35.315Z] 14:01:35    ERROR -  /builds/worker/checkouts/gecko/js/src/gc/Nursery.cpp:1492:47: error: use of undeclared identifier 'isnan'
[task 2020-06-08T14:01:35.315Z] 14:01:35     INFO -    MOZ_ASSERT(!isnan(value) && !isnan(min) && !isnan(max));
[task 2020-06-08T14:01:35.315Z] 14:01:35     INFO -                                                ^
[task 2020-06-08T14:01:35.315Z] 14:01:35     INFO -  6 errors generated.
[task 2020-06-08T14:01:35.315Z] 14:01:35    ERROR -  make[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:748: Unified_cpp_js_src_gc1.o] Error 1
[task 2020-06-08T14:01:35.315Z] 14:01:35     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/js/src/gc'
[task 2020-06-08T14:01:35.316Z] 14:01:35    ERROR -  make[3]: *** [/builds/worker/checkouts/gecko/config/recurse.mk:74: js/src/gc/target-objects] Error 2
[task 2020-06-08T14:01:35.316Z] 14:01:35     INFO -  make[3]: *** Waiting for unfinished jobs....
Flags: needinfo?(jcoppeard)
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0888757e4d2a Refactor nursery to update previous collection data in one place r=sfink https://hg.mozilla.org/integration/autoland/rev/c618fdb3c556 Refactor nursery to move all telemetry to one method r=sfink https://hg.mozilla.org/integration/autoland/rev/03f88a7cd040 Refactor nursery to use double for floating point variable everywhere r=sfink https://hg.mozilla.org/integration/autoland/rev/fdb6eafa138d Round nursery size parameters once when they are set rather than on every use r=sfink https://hg.mozilla.org/integration/autoland/rev/4300fe993e2f Refactor nursery resizing r=sfink
Regressions: 1644258
Flags: needinfo?(jcoppeard)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: