Skip to content

Commit

Permalink
Use references for vm::Runtime and its base classes
Browse files Browse the repository at this point in the history 
Summary:
References are immutable and non-nullable, which is useful for
readability, and more importantly, as a hint to the compiler.

`vm::Runtime` has two non-empty base classes, `PointerBase` and
`HandleRootOwner`, which we frequently cast to from `Runtime*`. When
casting to a base class that is at an offset from the start of an
object (in this case `PointerBase`), C++ requires that a nullptr remain
a nullptr, so the compiler needs to emit an extra test + cmov to check
for nullptrs each time we cast. The compiler is not very good at
optimising these away, so instead, we should just pass `Runtime` by
reference.

Reviewed By: kodafb

Differential Revision: D34501831

fbshipit-source-id: 658f4d2ebe2803ff02915b30448da54ed84922d0
  • Loading branch information
@neildhar @facebook-github-bot
neildhar authored and facebook-github-bot committed Mar 8, 2022
1 parent 7151287 commit 6cf9084
Show file tree
Hide file tree
Showing 214 changed files with 5,261 additions and 5,334 deletions.
156 changes: 77 additions & 79 deletions API/hermes/hermes.cpp
View file

Large diffs are not rendered by default.

15 changes: 7 additions & 8 deletions include/hermes/ConsoleHost/ConsoleHost.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class ConsoleHostContext {

public:
/// Registers the ConsoleHostContext roots with \p runtime.
ConsoleHostContext(vm::Runtime *runtime);
ConsoleHostContext(vm::Runtime &runtime);

/// \return true when there are no queued tasks remaining.
bool tasksEmpty() const {
Expand Down Expand Up @@ -91,14 +91,13 @@ namespace microtask {
/// Complete a checkpoint by repetitively trying to drain the engine job queue
/// until there was no errors (implying queue exhaustiveness).
/// Note that exceptions are directly printed to stderr.
inline void performCheckpoint(vm::Runtime *runtime) {
if (!runtime->useJobQueue())
inline void performCheckpoint(vm::Runtime &runtime) {
if (!runtime.useJobQueue())
return;

while (
LLVM_UNLIKELY(runtime->drainJobs() == vm::ExecutionStatus::EXCEPTION)) {
runtime->printException(
llvh::errs(), runtime->makeHandle(runtime->getThrownValue()));
while (LLVM_UNLIKELY(runtime.drainJobs() == vm::ExecutionStatus::EXCEPTION)) {
runtime.printException(
llvh::errs(), runtime.makeHandle(runtime.getThrownValue()));
};
}

Expand All @@ -116,7 +115,7 @@ inline void performCheckpoint(vm::Runtime *runtime) {
/// \p filename If non-null, the filename of the BC buffer being loaded.
/// Used to find the other segments to be loaded at runtime.
void installConsoleBindings(
vm::Runtime *runtime,
vm::Runtime &runtime,
ConsoleHostContext &ctx,
vm::StatSamplingThread *statSampler = nullptr,
const std::string *filename = nullptr);
Expand Down
18 changes: 9 additions & 9 deletions include/hermes/Platform/Intl/PlatformIntl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,14 @@ using Part = std::unordered_map<std::u16string, std::u16string>;
/// as anybody depending on it is trying too hard.

vm::CallResult<std::vector<std::u16string>> getCanonicalLocales(
vm::Runtime *runtime,
vm::Runtime &runtime,
const std::vector<std::u16string> &locales);
vm::CallResult<std::u16string> toLocaleLowerCase(
vm::Runtime *runtime,
vm::Runtime &runtime,
const std::vector<std::u16string> &locales,
const std::u16string &str);
vm::CallResult<std::u16string> toLocaleUpperCase(
vm::Runtime *runtime,
vm::Runtime &runtime,
const std::vector<std::u16string> &locales,
const std::u16string &str);

Expand All @@ -99,12 +99,12 @@ class Collator : public vm::DecoratedObject::Decoration {
~Collator();

static vm::CallResult<std::vector<std::u16string>> supportedLocalesOf(
vm::Runtime *runtime,
vm::Runtime &runtime,
const std::vector<std::u16string> &locales,
const Options &options) noexcept;

vm::ExecutionStatus initialize(
vm::Runtime *runtime,
vm::Runtime &runtime,
const std::vector<std::u16string> &locales,
const Options &options) noexcept;
Options resolvedOptions() noexcept;
Expand All @@ -122,12 +122,12 @@ class DateTimeFormat : public vm::DecoratedObject::Decoration {
~DateTimeFormat();

static vm::CallResult<std::vector<std::u16string>> supportedLocalesOf(
vm::Runtime *runtime,
vm::Runtime &runtime,
const std::vector<std::u16string> &locales,
const Options &options) noexcept;

vm::ExecutionStatus initialize(
vm::Runtime *runtime,
vm::Runtime &runtime,
const std::vector<std::u16string> &locales,
const Options &options) noexcept;
Options resolvedOptions() noexcept;
Expand All @@ -146,12 +146,12 @@ class NumberFormat : public vm::DecoratedObject::Decoration {
~NumberFormat();

static vm::CallResult<std::vector<std::u16string>> supportedLocalesOf(
vm::Runtime *runtime,
vm::Runtime &runtime,
const std::vector<std::u16string> &locales,
const Options &options) noexcept;

vm::ExecutionStatus initialize(
vm::Runtime *runtime,
vm::Runtime &runtime,
const std::vector<std::u16string> &locales,
const Options &options) noexcept;
Options resolvedOptions() noexcept;
Expand Down
6 changes: 3 additions & 3 deletions include/hermes/VM/ArrayLike.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ namespace vm {
/// Returns the length of the List
inline CallResult<uint64_t> getArrayLikeLength(
Handle<JSObject> arrayLikeHandle,
Runtime *runtime) {
Runtime &runtime) {
auto propRes = JSObject::getNamed_RJS(
arrayLikeHandle, runtime, Predefined::getSymbolID(Predefined::length));
if (LLVM_UNLIKELY(propRes == ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}
return toLengthU64(runtime, runtime->makeHandle(std::move(*propRes)));
return toLengthU64(runtime, runtime.makeHandle(std::move(*propRes)));
}

/// ES9 7.3.17 CreateListFromArrayLike
Expand All @@ -38,7 +38,7 @@ inline CallResult<uint64_t> getArrayLikeLength(
template <typename ElementCB>
ExecutionStatus createListFromArrayLike(
Handle<JSObject> arrayLikeHandle,
Runtime *runtime,
Runtime &runtime,
uint64_t length,
const ElementCB &elementCB) {
GCScope gcScope(runtime);
Expand Down
40 changes: 20 additions & 20 deletions include/hermes/VM/ArrayStorage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,13 @@ class ArrayStorageBase final
}

/// Create a new instance with at least the specified \p capacity.
static CallResult<HermesValue> create(Runtime *runtime, size_type capacity) {
static CallResult<HermesValue> create(Runtime &runtime, size_type capacity) {
if (LLVM_UNLIKELY(capacity > maxElements())) {
return throwExcessiveCapacityError(runtime, capacity);
}
const auto allocSize = allocationSize(capacity);
auto *cell = runtime->makeAVariable<ArrayStorageBase<HVType>>(
allocSize, &runtime->getHeap(), allocSize);
auto *cell = runtime.makeAVariable<ArrayStorageBase<HVType>>(
allocSize, &runtime.getHeap(), allocSize);
return HermesValue::encodeObjectValue(cell);
}

Expand All @@ -102,23 +102,23 @@ class ArrayStorageBase final

/// Create a new long-lived instance with at least the specified \p capacity.
static CallResult<HermesValue> createLongLived(
Runtime *runtime,
Runtime &runtime,
size_type capacity) {
if (LLVM_UNLIKELY(capacity > maxElements())) {
return throwExcessiveCapacityError(runtime, capacity);
}
const auto allocSize = allocationSize(capacity);
return HermesValue::encodeObjectValue(
runtime->makeAVariable<
runtime.makeAVariable<
ArrayStorageBase<HVType>,
HasFinalizer::No,
LongLived::Yes>(allocSize, &runtime->getHeap(), allocSize));
LongLived::Yes>(allocSize, &runtime.getHeap(), allocSize));
}

/// Create a new instance with at least the specified \p capacity and a size
/// of \p size. Requires that \p size <= \p capacity.
static CallResult<HermesValue>
create(Runtime *runtime, size_type capacity, size_type size) {
create(Runtime &runtime, size_type capacity, size_type size) {
auto arrRes = create(runtime, capacity);
if (LLVM_UNLIKELY(arrRes == ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
Expand Down Expand Up @@ -180,7 +180,7 @@ class ArrayStorageBase final
/// Append the given element to the end (increasing size by 1).
static ExecutionStatus push_back(
MutableHandle<ArrayStorageBase<HVType>> &selfHandle,
Runtime *runtime,
Runtime &runtime,
Handle<> value) {
auto *self = selfHandle.get();
const auto currSz = self->size();
Expand All @@ -195,22 +195,22 @@ class ArrayStorageBase final
if (LLVM_LIKELY(currSz < self->capacity())) {
// Use the constructor of GCHermesValue to use the correct write barrier
// for uninitialized memory.
new (&self->data()[currSz]) GCHVType(hv, &runtime->getHeap());
new (&self->data()[currSz]) GCHVType(hv, &runtime.getHeap());
self->size_.store(currSz + 1, std::memory_order_release);
return ExecutionStatus::RETURNED;
}
return pushBackSlowPath(selfHandle, runtime, value);
}

/// Pop the last element off the array and return it.
HVType pop_back(Runtime *runtime) {
HVType pop_back(Runtime &runtime) {
const size_type sz = size();
assert(sz > 0 && "Can't pop from empty ArrayStorage");
HVType val = data()[sz - 1];
// In Hades, a snapshot write barrier must be executed on the value that is
// conceptually being changed to null. The write doesn't need to occur, but
// it is the only correct way to use the write barrier.
data()[sz - 1].unreachableWriteBarrier(&runtime->getHeap());
data()[sz - 1].unreachableWriteBarrier(&runtime.getHeap());
// The background thread can't mutate size, so we don't need fetch_sub here.
// Relaxed is fine, because the GC doesn't care about the order of seeing
// the length and the individual elements, as long as illegal HermesValues
Expand All @@ -223,15 +223,15 @@ class ArrayStorageBase final
/// reallocating if needed.
static ExecutionStatus ensureCapacity(
MutableHandle<ArrayStorageBase<HVType>> &selfHandle,
Runtime *runtime,
Runtime &runtime,
size_type capacity);

/// Change the size of the storage to \p newSize. This can increase the size
/// (in which case the new elements will be initialized to empty), or decrease
/// the size.
static ExecutionStatus resize(
MutableHandle<ArrayStorageBase<HVType>> &selfHandle,
Runtime *runtime,
Runtime &runtime,
size_type newSize) {
return shift(selfHandle, runtime, 0, 0, newSize);
}
Expand All @@ -244,7 +244,7 @@ class ArrayStorageBase final
/// as \c resize.
static ExecutionStatus resizeLeft(
MutableHandle<ArrayStorageBase<HVType>> &selfHandle,
Runtime *runtime,
Runtime &runtime,
size_type newSize) {
return shift(selfHandle, runtime, 0, newSize - selfHandle->size(), newSize);
}
Expand All @@ -259,9 +259,9 @@ class ArrayStorageBase final

static void resizeWithinCapacity(
ArrayStorageBase<HVType> *self,
Runtime *runtime,
Runtime &runtime,
size_type newSize) {
resizeWithinCapacity(self, &runtime->getHeap(), newSize);
resizeWithinCapacity(self, &runtime.getHeap(), newSize);
}

private:
Expand All @@ -280,14 +280,14 @@ class ArrayStorageBase final
/// capacity allocated, and the max that is allowed.
/// \returns ExecutionStatus::EXCEPTION always.
static ExecutionStatus throwExcessiveCapacityError(
Runtime *runtime,
Runtime &runtime,
size_type capacity);

/// Append the given element to the end when the capacity has been exhausted
/// and a reallocation is needed.
static ExecutionStatus pushBackSlowPath(
MutableHandle<ArrayStorageBase<HVType>> &selfHandle,
Runtime *runtime,
Runtime &runtime,
Handle<> value);

/// Shrinks \p self during GC compaction, so that it's capacity is equal to
Expand All @@ -302,7 +302,7 @@ class ArrayStorageBase final
/// "length" number of elements are copied from "fromFirst" to "toFirst".
static ExecutionStatus reallocateToLarger(
MutableHandle<ArrayStorageBase<HVType>> &selfHandle,
Runtime *runtime,
Runtime &runtime,
size_type capacity,
size_type fromFirst,
size_type toFirst,
Expand All @@ -325,7 +325,7 @@ class ArrayStorageBase final
/// "empty".
static ExecutionStatus shift(
MutableHandle<ArrayStorageBase<HVType>> &selfHandle,
Runtime *runtime,
Runtime &runtime,
size_type fromFirst,
size_type toFirst,
size_type toLast);
Expand Down
8 changes: 4 additions & 4 deletions include/hermes/VM/BoxedDouble.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ class BoxedDouble final : public GCCell {
return cell->getKind() == CellKind::BoxedDoubleKind;
}

static BoxedDouble *create(double d, Runtime *runtime) {
return runtime->makeAFixed<BoxedDouble>(d, runtime);
static BoxedDouble *create(double d, Runtime &runtime) {
return runtime.makeAFixed<BoxedDouble>(d, runtime);
}

BoxedDouble(double d, Runtime *runtime)
: GCCell(&runtime->getHeap(), &vt), value_(d) {}
BoxedDouble(double d, Runtime &runtime)
: GCCell(&runtime.getHeap(), &vt), value_(d) {}

double get() const {
return value_;
Expand Down
Loading

0 comments on commit 6cf9084

Please sign in to comment.