Skip to content

Commit

Permalink
Remove ShouldRandomizeAllocSpace
Browse files Browse the repository at this point in the history 
Summary:
This is an unused debug only config that isn't honoured by either GC.
It was originally added to demonstrate bugs when eliding write barriers
in newly created objects. We could support it in Hades, but it wouldn't
serve much purpose, because Hades will always allocate things in the
young gen unless explicitly told not to.

Reviewed By: jpporto

Differential Revision: D38306384

fbshipit-source-id: 75c44603a23f3eaeb4a5d47fa68da7e5f2f89ea8
  • Loading branch information
@neildhar @facebook-github-bot
neildhar authored and facebook-github-bot committed Aug 1, 2022
1 parent 4716b60 commit bc7eafb
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 127 deletions.
8 changes: 0 additions & 8 deletions include/hermes/ConsoleHost/RuntimeFlags.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,6 @@ static opt<int64_t, false, RandomSeedParser> GCSanitizeRandomSeed(
#endif
);

static opt<bool> GCRandomizeAllocSpace(
"gc-randomize-alloc-space",
desc(
"For GC's, like GenGC, that can allocate in different spaces, randomize "
"the choice of space."),
cat(GCCategory),
init(false));

static opt<MemorySize, false, MemorySizeParser> MinHeapSize(
"gc-min-heap",
desc("Minimum heap size. Format: <unsigned>{{K,M,G}{iB}"),
Expand Down
14 changes: 0 additions & 14 deletions include/hermes/VM/GCBase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -796,12 +796,6 @@ class GCBase {
class... Args>
T *makeA(uint32_t size, Args &&...args);

/// \return true if the "target space" for allocations should be randomized
/// (for GCs where that concept makes sense).
bool shouldRandomizeAllocSpace() const {
return randomizeAllocSpace_;
}

/// Name to identify this heap in logs.
const std::string &getName() const {
return name_;
Expand Down Expand Up @@ -1575,14 +1569,6 @@ class GCBase {

/// True if the tripwire has already been called on this heap.
bool tripwireCalled_{false};

/// Whether to randomize the "target space" for allocations, for GC's in which
/// this concept makes sense. Only available in debug builds.
#ifndef NDEBUG
bool randomizeAllocSpace_{false};
#else
static const bool randomizeAllocSpace_{false};
#endif
};

#ifdef HERMESVM_EXCEPTION_ON_OOM
Expand Down
7 changes: 1 addition & 6 deletions lib/VM/GCBase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,7 @@ GCBase::GCBase(
sanitizeRate_(gcConfig.getSanitizeConfig().getSanitizeRate()),
#endif
tripwireCallback_(gcConfig.getTripwireConfig().getCallback()),
tripwireLimit_(gcConfig.getTripwireConfig().getLimit())
#ifndef NDEBUG
,
randomizeAllocSpace_(gcConfig.getShouldRandomizeAllocSpace())
#endif
{
tripwireLimit_(gcConfig.getTripwireConfig().getLimit()) {
for (unsigned i = 0; i < (unsigned)XorPtrKeyID::_NumKeys; ++i) {
pointerEncryptionKey_[i] = std::random_device()();
if constexpr (sizeof(uintptr_t) >= 8) {
Expand Down
123 changes: 60 additions & 63 deletions public/hermes/Public/GCConfig.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,69 +143,66 @@ enum class GCEventKind {

/// Parameters for GC Initialisation. Check documentation in README.md
/// constexpr indicates that the default value is constexpr.
#define GC_FIELDS(F) \
/* Minimum heap size hint. */ \
F(constexpr, gcheapsize_t, MinHeapSize, 0) \
\
/* Initial heap size hint. */ \
F(constexpr, gcheapsize_t, InitHeapSize, 32 << 20) \
\
/* Maximum heap size hint. */ \
F(constexpr, gcheapsize_t, MaxHeapSize, 3u << 30) \
\
/* Sizing heuristic: fraction of heap to be occupied by live data. */ \
F(constexpr, double, OccupancyTarget, 0.5) \
\
/* Number of consecutive full collections considered to be an OOM. */ \
F(constexpr, \
unsigned, \
EffectiveOOMThreshold, \
std::numeric_limits<unsigned>::max()) \
\
/* Sanitizer configuration for the GC. */ \
F(constexpr, GCSanitizeConfig, SanitizeConfig) \
\
/* Whether the GC should spread allocations across all its "spaces". */ \
F(constexpr, bool, ShouldRandomizeAllocSpace, false) \
\
/* Whether to Keep track of GC Statistics. */ \
F(constexpr, bool, ShouldRecordStats, false) \
\
/* How aggressively to return unused memory to the OS. */ \
F(constexpr, ReleaseUnused, ShouldReleaseUnused, kReleaseUnusedOld) \
\
/* Name for this heap in logs. */ \
F(HERMES_NON_CONSTEXPR, std::string, Name, "") \
\
/* Configuration for the Heap Tripwire. */ \
F(HERMES_NON_CONSTEXPR, GCTripwireConfig, TripwireConfig) \
\
/* Whether to (initially) allocate from the young gen (true) or the */ \
/* old gen (false). */ \
F(constexpr, bool, AllocInYoung, true) \
\
/* Whether to fill the YG with invalid data after each collection. */ \
F(constexpr, bool, OverwriteDeadYGObjects, false) \
\
/* Whether to revert, if necessary, to young-gen allocation at TTI. */ \
F(constexpr, bool, RevertToYGAtTTI, false) \
\
/* Whether to use mprotect on GC metadata between GCs. */ \
F(constexpr, bool, ProtectMetadata, false) \
\
/* Callout for an analytics event. */ \
F(HERMES_NON_CONSTEXPR, \
std::function<void(const GCAnalyticsEvent &)>, \
AnalyticsCallback, \
nullptr) \
\
/* Called at GC events (see GCEventKind enum for the list). The */ \
/* second argument contains human-readable details about the event. */ \
/* NOTE: The function MUST NOT invoke any methods on the Runtime. */ \
F(HERMES_NON_CONSTEXPR, \
std::function<void(GCEventKind, const char *)>, \
Callback, \
nullptr) \
#define GC_FIELDS(F) \
/* Minimum heap size hint. */ \
F(constexpr, gcheapsize_t, MinHeapSize, 0) \
\
/* Initial heap size hint. */ \
F(constexpr, gcheapsize_t, InitHeapSize, 32 << 20) \
\
/* Maximum heap size hint. */ \
F(constexpr, gcheapsize_t, MaxHeapSize, 3u << 30) \
\
/* Sizing heuristic: fraction of heap to be occupied by live data. */ \
F(constexpr, double, OccupancyTarget, 0.5) \
\
/* Number of consecutive full collections considered to be an OOM. */ \
F(constexpr, \
unsigned, \
EffectiveOOMThreshold, \
std::numeric_limits<unsigned>::max()) \
\
/* Sanitizer configuration for the GC. */ \
F(constexpr, GCSanitizeConfig, SanitizeConfig) \
\
/* Whether to Keep track of GC Statistics. */ \
F(constexpr, bool, ShouldRecordStats, false) \
\
/* How aggressively to return unused memory to the OS. */ \
F(constexpr, ReleaseUnused, ShouldReleaseUnused, kReleaseUnusedOld) \
\
/* Name for this heap in logs. */ \
F(HERMES_NON_CONSTEXPR, std::string, Name, "") \
\
/* Configuration for the Heap Tripwire. */ \
F(HERMES_NON_CONSTEXPR, GCTripwireConfig, TripwireConfig) \
\
/* Whether to (initially) allocate from the young gen (true) or the */ \
/* old gen (false). */ \
F(constexpr, bool, AllocInYoung, true) \
\
/* Whether to fill the YG with invalid data after each collection. */ \
F(constexpr, bool, OverwriteDeadYGObjects, false) \
\
/* Whether to revert, if necessary, to young-gen allocation at TTI. */ \
F(constexpr, bool, RevertToYGAtTTI, false) \
\
/* Whether to use mprotect on GC metadata between GCs. */ \
F(constexpr, bool, ProtectMetadata, false) \
\
/* Callout for an analytics event. */ \
F(HERMES_NON_CONSTEXPR, \
std::function<void(const GCAnalyticsEvent &)>, \
AnalyticsCallback, \
nullptr) \
\
/* Called at GC events (see GCEventKind enum for the list). The */ \
/* second argument contains human-readable details about the event. */ \
/* NOTE: The function MUST NOT invoke any methods on the Runtime. */ \
F(HERMES_NON_CONSTEXPR, \
std::function<void(GCEventKind, const char *)>, \
Callback, \
nullptr) \
/* GC_FIELDS END */

_HERMES_CTORCONFIG_STRUCT(GCConfig, GC_FIELDS, {
Expand Down
32 changes: 15 additions & 17 deletions tools/hermes/hermes.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,23 +82,21 @@ static int executeHBCBytecodeFromCL(
ExecuteOptions options;
options.runtimeConfig =
vm::RuntimeConfig::Builder()
.withGCConfig(
vm::GCConfig::Builder()
.withMinHeapSize(cl::MinHeapSize.bytes)
.withInitHeapSize(cl::InitHeapSize.bytes)
.withMaxHeapSize(cl::MaxHeapSize.bytes)
.withOccupancyTarget(cl::OccupancyTarget)
.withSanitizeConfig(
vm::GCSanitizeConfig::Builder()
.withSanitizeRate(cl::GCSanitizeRate)
.withRandomSeed(cl::GCSanitizeRandomSeed)
.build())
.withShouldRandomizeAllocSpace(cl::GCRandomizeAllocSpace)
.withShouldRecordStats(recStats)
.withShouldReleaseUnused(vm::kReleaseUnusedNone)
.withAllocInYoung(cl::GCAllocYoung)
.withRevertToYGAtTTI(cl::GCRevertToYGAtTTI)
.build())
.withGCConfig(vm::GCConfig::Builder()
.withMinHeapSize(cl::MinHeapSize.bytes)
.withInitHeapSize(cl::InitHeapSize.bytes)
.withMaxHeapSize(cl::MaxHeapSize.bytes)
.withOccupancyTarget(cl::OccupancyTarget)
.withSanitizeConfig(
vm::GCSanitizeConfig::Builder()
.withSanitizeRate(cl::GCSanitizeRate)
.withRandomSeed(cl::GCSanitizeRandomSeed)
.build())
.withShouldRecordStats(recStats)
.withShouldReleaseUnused(vm::kReleaseUnusedNone)
.withAllocInYoung(cl::GCAllocYoung)
.withRevertToYGAtTTI(cl::GCRevertToYGAtTTI)
.build())
.withEnableEval(cl::EnableEval)
.withVerifyEvalIR(cl::VerifyIR)
.withOptimizedEval(cl::OptimizedEval)
Expand Down
28 changes: 13 additions & 15 deletions tools/hvm/hvm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,21 +105,19 @@ int main(int argc, char **argv) {
ExecuteOptions options;
options.runtimeConfig =
vm::RuntimeConfig::Builder()
.withGCConfig(
vm::GCConfig::Builder()
.withInitHeapSize(cl::InitHeapSize.bytes)
.withMaxHeapSize(cl::MaxHeapSize.bytes)
.withSanitizeConfig(
vm::GCSanitizeConfig::Builder()
.withSanitizeRate(cl::GCSanitizeRate)
.withRandomSeed(cl::GCSanitizeRandomSeed)
.build())
.withShouldRandomizeAllocSpace(cl::GCRandomizeAllocSpace)
.withShouldRecordStats(
GCPrintStats && !cl::StableInstructionCount)
.withShouldReleaseUnused(vm::kReleaseUnusedNone)
.withName("hvm")
.build())
.withGCConfig(vm::GCConfig::Builder()
.withInitHeapSize(cl::InitHeapSize.bytes)
.withMaxHeapSize(cl::MaxHeapSize.bytes)
.withSanitizeConfig(
vm::GCSanitizeConfig::Builder()
.withSanitizeRate(cl::GCSanitizeRate)
.withRandomSeed(cl::GCSanitizeRandomSeed)
.build())
.withShouldRecordStats(
GCPrintStats && !cl::StableInstructionCount)
.withShouldReleaseUnused(vm::kReleaseUnusedNone)
.withName("hvm")
.build())
.withES6Promise(cl::ES6Promise)
.withES6Proxy(cl::ES6Proxy)
.withIntl(cl::Intl)
Expand Down
6 changes: 2 additions & 4 deletions unittests/VMRuntime/TestHelpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,8 @@ static constexpr uint32_t kInitHeapLarge = 1 << 20;
static constexpr uint32_t kMaxHeapLarge = 1 << 24;

static const GCConfig::Builder kTestGCConfigBaseBuilder =
GCConfig::Builder()
.withSanitizeConfig(
vm::GCSanitizeConfig::Builder().withSanitizeRate(0.0).build())
.withShouldRandomizeAllocSpace(false);
GCConfig::Builder().withSanitizeConfig(
vm::GCSanitizeConfig::Builder().withSanitizeRate(0.0).build());

static const GCConfig kTestGCConfigSmall =
GCConfig::Builder(kTestGCConfigBaseBuilder)
Expand Down

0 comments on commit bc7eafb

Please sign in to comment.