Skip to content

Commit

Permalink
Remove HeapCellIterator and AlignedHeapSegment::cells()
Browse files Browse the repository at this point in the history
Summary:
With this change, we can remove the dependency on GCCell.h in the
header.

Reviewed By: neildhar

Differential Revision: D66727534
  • Loading branch information
lavenzg authored and facebook-github-bot committed Dec 6, 2024
1 parent fcda87c commit acd95c1
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 41 deletions.
40 changes: 1 addition & 39 deletions include/hermes/VM/AlignedHeapSegment.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,17 @@
#include "hermes/Support/OSCompat.h"
#include "hermes/VM/AdviseUnused.h"
#include "hermes/VM/AllocResult.h"
#include "hermes/VM/AllocSource.h"
#include "hermes/VM/CardTableNC.h"
#include "hermes/VM/GCBase.h"
#include "hermes/VM/GCCell.h"
#include "hermes/VM/HeapAlign.h"

#include "llvh/Support/MathExtras.h"

#include <cstdint>
#include <vector>

namespace hermes {
namespace vm {

class GCCell;
class StorageProvider;

#ifndef HERMESVM_LOG_HEAP_SEGMENT_SIZE
Expand Down Expand Up @@ -191,30 +188,6 @@ class AlignedHeapSegment {
0,
"Guard page must be aligned to likely page size");

class HeapCellIterator : public llvh::iterator_facade_base<
HeapCellIterator,
std::forward_iterator_tag,
GCCell *> {
public:
HeapCellIterator(GCCell *cell) : cell_(cell) {}

bool operator==(const HeapCellIterator &R) const {
return cell_ == R.cell_;
}

GCCell *const &operator*() const {
return cell_;
}

HeapCellIterator &operator++() {
cell_ = cell_->nextCell();
return *this;
}

private:
GCCell *cell_{nullptr};
};

/// Returns the index of the segment containing \p lowLim, which is required
/// to be the start of its containing segment. (This can allow extra
/// efficiency, in cases where the segment start has already been computed.)
Expand Down Expand Up @@ -333,9 +306,6 @@ class AlignedHeapSegment {
/// Returns the address at which the next allocation, if any, will occur.
inline char *level() const;

/// Returns an iterator range corresponding to the cells in this segment.
inline llvh::iterator_range<HeapCellIterator> cells();

/// Returns whether \p a and \p b are contained in the same
/// AlignedHeapSegment.
inline static bool containedInSame(const void *a, const void *b);
Expand Down Expand Up @@ -425,7 +395,6 @@ class AlignedHeapSegment {

AllocResult AlignedHeapSegment::alloc(uint32_t size) {
assert(lowLim() != nullptr && "Cannot allocate in a null segment");
assert(size >= sizeof(GCCell) && "cell must be larger than GCCell");
assert(isSizeHeapAligned(size) && "size must be heap aligned");

char *cellPtr; // Initialized in the if below.
Expand Down Expand Up @@ -545,13 +514,6 @@ char *AlignedHeapSegment::level() const {
return level_;
}

llvh::iterator_range<AlignedHeapSegment::HeapCellIterator>
AlignedHeapSegment::cells() {
return {
HeapCellIterator(reinterpret_cast<GCCell *>(start())),
HeapCellIterator(reinterpret_cast<GCCell *>(level()))};
}

/* static */
bool AlignedHeapSegment::containedInSame(const void *a, const void *b) {
return (reinterpret_cast<uintptr_t>(a) ^ reinterpret_cast<uintptr_t>(b)) <
Expand Down
11 changes: 9 additions & 2 deletions lib/VM/gcs/HadesGC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,10 @@ template <typename CallbackFunction>
void HadesGC::forAllObjsInSegment(
hermes::vm::AlignedHeapSegment &seg,
CallbackFunction callback) {
for (GCCell *cell : seg.cells()) {
for (GCCell *cell = reinterpret_cast<GCCell *>(seg.start()),
*end = reinterpret_cast<GCCell *>(seg.level());
cell < end;
cell = cell->nextCell()) {
// Skip free-list entries.
if (!vmisa<OldGen::FreelistCell>(cell)) {
callback(cell);
Expand Down Expand Up @@ -1052,7 +1055,11 @@ bool HadesGC::OldGen::sweepNext(bool backgroundThread) {
char *freeRangeStart = nullptr, *freeRangeEnd = nullptr;
size_t mergedCells = 0;
int32_t segmentSweptBytes = 0;
for (GCCell *cell : segments_[sweepIterator_.segNumber].cells()) {
auto &seg = segments_[sweepIterator_.segNumber];
for (GCCell *cell = reinterpret_cast<GCCell *>(seg.start()),
*end = reinterpret_cast<GCCell *>(seg.level());
cell < end;
cell = cell->nextCell()) {
assert(cell->isValid() && "Invalid cell in sweeping");
if (AlignedHeapSegment::getCellMarkBit(cell)) {
// Cannot concurrently trim storage. Technically just checking
Expand Down

0 comments on commit acd95c1

Please sign in to comment.