From 7bd857f0429225079a0a789c21e3f3c4b6737a39 Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Mon, 22 May 2023 14:31:40 +0000 Subject: [PATCH] Bug 1833854 - Part 3: Allow min and max nursery sizes to be set regardless of current values r=sfink Currently we prevent setting a new minimum value that's greater than the current maximum value and vice versa. This is problematic because the caller may not know the current values. For example, they may want to set both min and max to values that are valid with respect to each other but not with resepect to existing values. (This is caused by the fact that the API only allows setting one value at a time but that's a larger issue.) Instead, handle this the way we do for all other parmeters. When setting a new value that invalidates an invariant with repsect to another parmeter, adjust that other parameter to maintain the invariant. Differential Revision: https://phabricator.services.mozilla.com/D178527 --- js/src/gc/Scheduling.cpp | 30 +++++++++++++++++++----------- js/src/gc/Scheduling.h | 2 ++ js/src/jit-test/tests/gc/bug-1531626.js | 32 +++++++++++++++++--------------- 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/js/src/gc/Scheduling.cpp b/js/src/gc/Scheduling.cpp index c816e948a2a3..00f3eda2a439 100644 --- a/js/src/gc/Scheduling.cpp +++ b/js/src/gc/Scheduling.cpp @@ -98,20 +98,14 @@ bool GCSchedulingTunables::setParameter(JSGCParamKey key, uint32_t value) { return false; } value = Nursery::roundSize(value); - if (value > gcMaxNurseryBytes_) { - return false; - } - gcMinNurseryBytes_ = value; + setMinNurseryBytes(value); break; case JSGC_MAX_NURSERY_BYTES: if (value < SystemPageSize() || value >= MaxNurseryBytesParam) { return false; } value = Nursery::roundSize(value); - if (value < gcMinNurseryBytes_) { - return false; - } - gcMaxNurseryBytes_ = value; + setMaxNurseryBytes(value); break; case JSGC_HIGH_FREQUENCY_TIME_LIMIT: highFrequencyThreshold_ = TimeDuration::FromMilliseconds(value); @@ -293,6 +287,20 @@ bool GCSchedulingTunables::kilobytesToBytes(uint32_t value, size_t* bytesOut) { return true; } +void GCSchedulingTunables::setMinNurseryBytes(size_t value) { + gcMinNurseryBytes_ = value; + if (gcMaxNurseryBytes_ < gcMinNurseryBytes_) { + gcMaxNurseryBytes_ = gcMinNurseryBytes_; + } +} + +void GCSchedulingTunables::setMaxNurseryBytes(size_t value) { + gcMaxNurseryBytes_ = value; + if (gcMinNurseryBytes_ > gcMaxNurseryBytes_) { + gcMinNurseryBytes_ = gcMaxNurseryBytes_; + } +} + void GCSchedulingTunables::setSmallHeapSizeMaxBytes(size_t value) { smallHeapSizeMaxBytes_ = value; if (smallHeapSizeMaxBytes_ >= largeHeapSizeMinBytes_) { @@ -337,10 +345,10 @@ void GCSchedulingTunables::resetParameter(JSGCParamKey key) { gcMaxBytes_ = TuningDefaults::GCMaxBytes; break; case JSGC_MIN_NURSERY_BYTES: + setMinNurseryBytes(TuningDefaults::GCMinNurseryBytes); + break; case JSGC_MAX_NURSERY_BYTES: - // Reset these togeather to maintain their min <= max invariant. - gcMinNurseryBytes_ = TuningDefaults::GCMinNurseryBytes; - gcMaxNurseryBytes_ = JS::DefaultNurseryMaxBytes; + setMaxNurseryBytes(JS::DefaultNurseryMaxBytes); break; case JSGC_HIGH_FREQUENCY_TIME_LIMIT: highFrequencyThreshold_ = diff --git a/js/src/gc/Scheduling.h b/js/src/gc/Scheduling.h index b8424655af82..304e841ec7c7 100644 --- a/js/src/gc/Scheduling.h +++ b/js/src/gc/Scheduling.h @@ -675,6 +675,8 @@ class GCSchedulingTunables { void resetParameter(JSGCParamKey key); private: + void setMinNurseryBytes(size_t value); + void setMaxNurseryBytes(size_t value); void setSmallHeapSizeMaxBytes(size_t value); void setLargeHeapSizeMinBytes(size_t value); void setHighFrequencySmallHeapGrowth(double value); diff --git a/js/src/jit-test/tests/gc/bug-1531626.js b/js/src/jit-test/tests/gc/bug-1531626.js index a3cca8dfd443..74351dc53e1b 100644 --- a/js/src/jit-test/tests/gc/bug-1531626.js +++ b/js/src/jit-test/tests/gc/bug-1531626.js @@ -9,42 +9,44 @@ load(libdir + "asserts.js"); const chunkSizeKB = gcparam('chunkBytes') / 1024; const pageSizeKB = gcparam('systemPageSizeKB'); -var testSizesKB = [128, 129, 255, 256, 516, 1023, 1024, 3*1024, 4*1024+1, 16*1024]; +const testSizesKB = [128, 129, 255, 256, 516, 1023, 1024, 3*1024, 4*1024+1, 16*1024]; // Valid maximum sizes must be >= 1MB. -var testMaxSizesKB = testSizesKB.filter(x => x >= 1024); +const testMaxSizesKB = testSizesKB.filter(x => x >= 1024); -for (var max of testMaxSizesKB) { +for (let max of testMaxSizesKB) { // Don't test minimums greater than the maximum. - for (var min of testSizesKB.filter(x => x <= max)) { + for (let min of testSizesKB.filter(x => x <= max)) { setMinMax(min, max); } } // The above loops raised the nursery size. Now reduce it to ensure that // forcibly-reducing it works correctly. -gcparam('minNurseryBytes', 256*1024); // need to avoid min > max; setMinMax(256, 1024); -// Try an invalid configuration. -assertErrorMessage( - () => setMinMax(2*1024, 1024), - Object, - "Parameter value out of range"); +// Try invalid configurations. +const badSizesKB = [ 0, 1, 128 * 1024 + 1]; +function assertParamOutOfRange(f) { + assertErrorMessage(f, Object, "Parameter value out of range"); +} +for (let size of badSizesKB) { + assertParamOutOfRange(() => gcparam('minNurseryBytes', size * 1024)); + assertParamOutOfRange(() => gcparam('maxNurseryBytes', size * 1024)); +} function setMinMax(min, max) { - // Set the maximum first so that we don't hit a case where max < min. - gcparam('maxNurseryBytes', max * 1024); gcparam('minNurseryBytes', min * 1024); - assertEq(gcparam('maxNurseryBytes'), nearestLegalSize(max) * 1024); + gcparam('maxNurseryBytes', max * 1024); assertEq(gcparam('minNurseryBytes'), nearestLegalSize(min) * 1024); + assertEq(gcparam('maxNurseryBytes'), nearestLegalSize(max) * 1024); allocateSomeThings(); gc(); } function allocateSomeThings() { - for (var i = 0; i < 1000; i++) { - var obj = { an: 'object', with: 'fields' }; + for (let i = 0; i < 1000; i++) { + let obj = { an: 'object', with: 'fields' }; } } -- 2.11.4.GIT