Skip to content

Commit

Permalink
Remove the non-Chrome memory profiler
Browse files Browse the repository at this point in the history
Summary:
Hermes had an older memory profiler implementation attempt that targeted a non-Chrome
memory profiler format. Now that there exists a Chrome-based format for a memory profiler
that doesn't re-use any of these mechanisms, it's better to delete the unused code.

Reviewed By: avp

Differential Revision: D21261119

fbshipit-source-id: cbe709854e0567e5435c7ddce04031f078d7fa53
  • Loading branch information
Riley Dulin authored and facebook-github-bot committed May 1, 2020
1 parent cdc4ea7 commit 6162d3a
Show file tree
Hide file tree
Showing 9 changed files with 3 additions and 224 deletions.
16 changes: 0 additions & 16 deletions include/hermes/VM/GCBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "hermes/Public/CrashManager.h"
#include "hermes/Public/GCConfig.h"
#include "hermes/Public/GCTripwireContext.h"
#include "hermes/Public/MemoryEventTracker.h"
#include "hermes/Support/CheckedMalloc.h"
#include "hermes/Support/OSCompat.h"
#include "hermes/Support/StatsAccumulator.h"
Expand Down Expand Up @@ -875,16 +874,6 @@ class GCBase {
inline uint64_t nextObjectID();
#endif

/// Get the instance of the memory event tracker. If memory
/// profiling is not enabled this should return nullptr.
inline MemoryEventTracker *memEventTracker() {
#ifdef HERMESVM_MEMORY_PROFILER
return memEventTracker_.get();
#else
return nullptr;
#endif
}

using TimePoint = std::chrono::steady_clock::time_point;
/// Return the difference between the two time points (end - start)
/// as a double representing the number of seconds in the duration.
Expand Down Expand Up @@ -1125,11 +1114,6 @@ class GCBase {
#endif

private:
#ifdef HERMESVM_MEMORY_PROFILER
/// Memory event tracker for the memory profiler
std::shared_ptr<MemoryEventTracker> memEventTracker_;
#endif

/// Callback called if it's not null when the Live Data Tripwire is triggered.
std::function<void(GCTripwireContext &)> tripwireCallback_;

Expand Down
29 changes: 2 additions & 27 deletions include/hermes/VM/GCCell.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,9 @@ class GCCell {
uint64_t _debugAllocationId_;
#endif

protected:
/// Single value enum that acts as a fill-in parameter to differentiate
/// GCCell constructors.
enum class AllocEventOption { DoNotEmit };

public:
explicit GCCell(GC *gc, const VTable *vtp);

/// GCCell constructor with extra 'fake' parameter 'opt' that lets us
/// differentiate from the other constructor. This constructor does not
/// emit an allocation event to the memory profiler when it is enabled.
explicit GCCell(GC *gc, const VTable *vtp, AllocEventOption doNotEmit);

// GCCell-s are not copyable (in the C++ sense).
GCCell(const GCCell &) = delete;
void operator=(const GCCell &) = delete;
Expand Down Expand Up @@ -241,10 +231,6 @@ class GCCell {
return getVT()->externalMemorySize(this);
}

protected:
/// Emit allocation event to memory profiler if it is enabled.
void trackAlloc(GC *gc, const VTable *vtp);

private:
/// This version assumes that the bit is set, and that it can
/// therefore subtract 1.
Expand All @@ -268,14 +254,12 @@ class VariableSizeRuntimeCell : public GCCell {
/// that same size for its lifetime.
/// To change the size, allocate a new object.
VariableSizeRuntimeCell(GC *gc, const VTable *vtp, uint32_t size)
: GCCell(gc, vtp, AllocEventOption::DoNotEmit),
variableSize_(heapAlignSize(size)) {
: GCCell(gc, vtp), variableSize_(heapAlignSize(size)) {
// Need to align to the GC here, since the GC doesn't know about this field.
assert(
size >= sizeof(VariableSizeRuntimeCell) &&
"Should not allocate a VariableSizeRuntimeCell of size less than "
"the size of a cell");
trackAlloc(gc, vtp);
}

public:
Expand Down Expand Up @@ -304,16 +288,7 @@ static_assert(
"GCCell's alignment exceeds the alignment requirement of the heap");

#ifdef NDEBUG
inline GCCell::GCCell(GC *gc, const VTable *vtp) : vtp_(vtp) {
trackAlloc(gc, vtp);
}

inline GCCell::GCCell(GC *gc, const VTable *vtp, AllocEventOption doNotEmit)
: vtp_(vtp) {}
#endif

#ifndef HERMESVM_MEMORY_PROFILER
inline void GCCell::trackAlloc(GC *gc, const VTable *vtp) {}
inline GCCell::GCCell(GC *, const VTable *vtp) : vtp_(vtp) {}
#endif

inline uint32_t GCCell::getAllocatedSize(const VTable *vtp) const {
Expand Down
3 changes: 0 additions & 3 deletions lib/VM/GCBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ GCBase::GCBase(
inGC_(false),
name_(gcConfig.getName()),
allocationLocationTracker_(this),
#ifdef HERMESVM_MEMORY_PROFILER
memEventTracker_(gcConfig.getMemEventTracker()),
#endif
tripwireCallback_(gcConfig.getTripwireConfig().getCallback()),
tripwireLimit_(gcConfig.getTripwireConfig().getLimit())
#ifdef HERMESVM_SANITIZE_HANDLES
Expand Down
15 changes: 0 additions & 15 deletions lib/VM/GCCell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,13 @@ namespace vm {

#ifndef NDEBUG
GCCell::GCCell(GC *gc, const VTable *vtp)
: GCCell(gc, vtp, GCCell::AllocEventOption::DoNotEmit) {
trackAlloc(gc, vtp);
}

GCCell::GCCell(GC *gc, const VTable *vtp, AllocEventOption doNotEmit)
: vtp_(vtp), _debugAllocationId_(gc->nextObjectID()) {
// If the vtp has a finalizer, then it should be the most recent thing
// added to the finalizer list.
assert(
(vtp->finalize_ == nullptr || gc->isMostRecentFinalizableObj(this)) &&
"If the vtp has a finalizer, then the obj should be on the "
"finalizer list");
#ifndef NDEBUG
if (gc->lastAllocationWasFixedSize() == GCBase::FixedSizeValue::Yes) {
assert(
vtp->size > 0 &&
Expand All @@ -35,20 +29,11 @@ GCCell::GCCell(GC *gc, const VTable *vtp, AllocEventOption doNotEmit)
vtp->size == 0 &&
"Variable size allocation must have object size = 0 in vtable.");
}
#endif
assert(
(!vtp->mallocSize_ || vtp->finalize_) &&
"If a cell uses malloc, then it needs a finalizer");
}
#endif

#ifdef HERMESVM_MEMORY_PROFILER
void GCCell::trackAlloc(GC *gc, const VTable *vtp) {
if (auto *met = gc->memEventTracker()) {
met->emitAlloc(static_cast<uint32_t>(vtp->kind), getAllocatedSize());
}
}
#endif

} // namespace vm
} // namespace hermes
7 changes: 0 additions & 7 deletions public/hermes/Public/GCConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#include "hermes/Public/CtorConfig.h"
#include "hermes/Public/GCTripwireContext.h"
#include "hermes/Public/MemoryEventTracker.h"

#include <algorithm>
#include <cassert>
Expand Down Expand Up @@ -141,12 +140,6 @@ enum class GCEventKind {
/* Whether to track allocation traces starting in the Runtime ctor. */ \
F(constexpr, bool, AllocationLocationTrackerFromStart, false) \
\
/* Pointer to the memory profiler (Memory Event Tracker). */ \
F(HERMES_NON_CONSTEXPR, \
std::shared_ptr<MemoryEventTracker>, \
MemEventTracker, \
nullptr) \
\
/* Callout for an analytics event. */ \
F(HERMES_NON_CONSTEXPR, \
std::function<void(const GCAnalyticsEvent &)>, \
Expand Down
30 changes: 0 additions & 30 deletions public/hermes/Public/MemoryEventTracker.h

This file was deleted.

1 change: 0 additions & 1 deletion unittests/VMRuntime/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ set(RTSources
NativeFrameTest.cpp
NativeFunctionNameTest.cpp
MarkBitArrayNCTest.cpp
MemoryEventTrackerTest.cpp
ObjectBufferTest.cpp
ObjectModelTest.cpp
OperationsTest.cpp
Expand Down
123 changes: 0 additions & 123 deletions unittests/VMRuntime/MemoryEventTrackerTest.cpp

This file was deleted.

3 changes: 1 addition & 2 deletions unittests/VMRuntime/TestHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,7 @@ struct DummyRuntime final : public HandleRootOwner,

/// A DummyRuntimeTestFixtureBase should be used by any test that requires a
/// DummyRuntime. It takes a metadata table and a GCConfig, the latter can be
/// used to specify heap size using the constants i.e kInitHeapSize and to
/// specify a MemoryEventTracker implementation for testing memory profiling.
/// used to specify heap size using the constants i.e kInitHeapSize.
class DummyRuntimeTestFixtureBase : public ::testing::Test {
std::shared_ptr<DummyRuntime> rt;

Expand Down

0 comments on commit 6162d3a

Please sign in to comment.