[vm, gc] Remove BumpAllocationScope.

This scope can sneak objects larger than kAllocatablePageSize directly into non-large heap pages.

This scope also holds a heap lock outside of the GC, which is likely to create issues when multiple isolates run on the same heap.

Change-Id: Ic56277c83ffccfe2e1396ed773dd30b8d172bbba
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/134040
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
This commit is contained in:
Ryan Macnak 2020-02-03 17:22:05 +00:00 committed by commit-bot@chromium.org
parent d9a0359843
commit 24418e6c6d
6 changed files with 1 additions and 72 deletions

View file

@ -1176,11 +1176,6 @@ Isolate* CreateWithinExistingIsolateGroup(IsolateGroup* group,
HANDLESCOPE(T);
StackZone zone(T);
// The kernel loader is about to allocate a bunch of new libraries,
// classes and functions into old space. Force growth, and use of the
// bump allocator instead of freelists.
BumpAllocateScope bump_allocate_scope(T);
// NOTE: We do not attach a finalizer for this object, because the
// embedder will free it once the isolate group has shutdown.
const auto& td = ExternalTypedData::Handle(ExternalTypedData::New(
@ -5174,11 +5169,6 @@ DART_EXPORT Dart_Handle Dart_LoadScriptFromKernel(const uint8_t* buffer,
CHECK_CALLBACK_STATE(T);
CHECK_COMPILATION_ALLOWED(I);
// The kernel loader is about to allocate a bunch of new libraries, classes,
// and functions into old space. Force growth, and use of the bump allocator
// instead of freelists.
BumpAllocateScope bump_allocate_scope(T);
// NOTE: We do not attach a finalizer for this object, because the embedder
// will free it once the isolate group has shutdown.
const auto& td = ExternalTypedData::Handle(ExternalTypedData::New(
@ -5422,11 +5412,6 @@ DART_EXPORT Dart_Handle Dart_LoadLibraryFromKernel(const uint8_t* buffer,
CHECK_CALLBACK_STATE(T);
CHECK_COMPILATION_ALLOWED(I);
// The kernel loader is about to allocate a bunch of new libraries, classes,
// and functions into old space. Force growth, and use of the bump allocator
// instead of freelists.
BumpAllocateScope bump_allocate_scope(T);
// NOTE: We do not attach a finalizer for this object, because the embedder
// will/should free it once the isolate group has shutdown.
// See also http://dartbug.com/37030.
@ -5491,11 +5476,6 @@ DART_EXPORT Dart_Handle Dart_FinalizeLoading(bool complete_futures) {
Isolate* I = T->isolate();
CHECK_CALLBACK_STATE(T);
// The kernel loader is about to allocate a bunch of new libraries, classes,
// and functions into old space. Force growth, and use of the bump allocator
// instead of freelists.
BumpAllocateScope bump_allocate_scope(T);
// Finalize all classes if needed.
Dart_Handle state = Api::CheckAndFinalizePendingClasses(T);
if (Api::IsError(state)) {

View file

@ -1139,22 +1139,4 @@ WritableCodePages::~WritableCodePages() {
isolate_->heap()->WriteProtectCode(true);
}
BumpAllocateScope::BumpAllocateScope(Thread* thread)
: ThreadStackResource(thread), no_reload_scope_(thread->isolate(), thread) {
ASSERT(!thread->bump_allocate());
// If the background compiler thread is not disabled, there will be a cycle
// between the symbol table lock and the old space data lock.
BackgroundCompiler::Disable(thread->isolate());
thread->heap()->WaitForMarkerTasks(thread);
thread->heap()->old_space()->AcquireDataLock();
thread->set_bump_allocate(true);
}
BumpAllocateScope::~BumpAllocateScope() {
ASSERT(thread()->bump_allocate());
thread()->set_bump_allocate(false);
thread()->heap()->old_space()->ReleaseDataLock();
BackgroundCompiler::Enable(thread()->isolate());
}
} // namespace dart

View file

@ -486,26 +486,6 @@ class WritableCodePages : StackResource {
Isolate* isolate_;
};
// This scope forces heap growth, forces use of the bump allocator, and
// takes the page lock. It is useful e.g. at program startup when allocating
// many objects into old gen (like libraries, classes, and functions).
class BumpAllocateScope : ThreadStackResource {
public:
explicit BumpAllocateScope(Thread* thread);
~BumpAllocateScope();
private:
// This is needed to avoid a GC while we hold the page lock, which would
// trigger a deadlock.
NoHeapGrowthControlScope no_growth_control_;
// A reload will try to allocate into new gen, which could trigger a
// scavenge and deadlock.
NoReloadScope no_reload_scope_;
DISALLOW_COPY_AND_ASSIGN(BumpAllocateScope);
};
#if defined(TESTING)
class GCTestHelper : public AllStatic {
public:

View file

@ -2504,15 +2504,7 @@ RawObject* Object::Allocate(intptr_t cls_id, intptr_t size, Heap::Space space) {
ASSERT(thread->no_callback_scope_depth() == 0);
Heap* heap = thread->heap();
uword address;
// In a bump allocation scope, all allocations go into old space.
if (thread->bump_allocate() && (space != Heap::kCode)) {
DEBUG_ASSERT(heap->old_space()->CurrentThreadOwnsDataLock());
address = heap->old_space()->TryAllocateDataBumpLocked(size);
} else {
address = heap->Allocate(size, space);
}
uword address = heap->Allocate(size, space);
if (UNLIKELY(address == 0)) {
if (thread->top_exit_frame_info() != 0) {
// Use the preallocated out of memory exception to avoid calling

View file

@ -97,7 +97,6 @@ Thread::Thread(bool is_vm_isolate)
deferred_interrupts_mask_(0),
deferred_interrupts_(0),
stack_overflow_count_(0),
bump_allocate_(false),
hierarchy_info_(NULL),
type_usage_info_(NULL),
pending_functions_(GrowableObjectArray::null()),

View file

@ -493,9 +493,6 @@ class Thread : public ThreadState {
static intptr_t top_offset() { return OFFSET_OF(Thread, top_); }
static intptr_t end_offset() { return OFFSET_OF(Thread, end_); }
bool bump_allocate() const { return bump_allocate_; }
void set_bump_allocate(bool b) { bump_allocate_ = b; }
int32_t no_safepoint_scope_depth() const {
#if defined(DEBUG)
return no_safepoint_scope_depth_;
@ -941,7 +938,6 @@ class Thread : public ThreadState {
uint16_t deferred_interrupts_mask_;
uint16_t deferred_interrupts_;
int32_t stack_overflow_count_;
bool bump_allocate_;
// Compiler state:
CompilerState* compiler_state_ = nullptr;