Commit 8d7522bc authored by Adam Klein's avatar Adam Klein Committed by Commit Bot

Revert "[heap] Remove page header tag from owner field."

This reverts commit 6af43874.

Reason for revert: Linux TSAN failures:

https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20TSAN/builds/19028

Original change's description:
> [heap] Remove page header tag from owner field.
> 
> Bug: chromium:800251
> Change-Id: I101131b4651b0bb27a79e5107ee43caf1229ffc7
> Reviewed-on: https://chromium-review.googlesource.com/860010
> Commit-Queue: Hannes Payer <hpayer@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#50507}

TBR=ulan@chromium.org,hpayer@chromium.org

Change-Id: I29001423959f6d9faadbdba5228b28cfb1f5b341
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:800251
Reviewed-on: https://chromium-review.googlesource.com/861923Reviewed-by: 's avatarAdam Klein <adamk@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50512}
parent e1f676ec
...@@ -413,10 +413,16 @@ const int kCodeAlignmentBits = 5; ...@@ -413,10 +413,16 @@ const int kCodeAlignmentBits = 5;
const intptr_t kCodeAlignment = 1 << kCodeAlignmentBits; const intptr_t kCodeAlignment = 1 << kCodeAlignmentBits;
const intptr_t kCodeAlignmentMask = kCodeAlignment - 1; const intptr_t kCodeAlignmentMask = kCodeAlignment - 1;
// Weak references are tagged using the second bit in a pointer. // The owner field of a page is tagged with the page header tag. We need that
const int kWeakReferenceTag = 3; // to find out if a slot is part of a large object. If we mask out the lower
const int kWeakReferenceTagSize = 2; // 0xfffff bits (1M pages), go to the owner offset, and see that this field
const intptr_t kWeakReferenceTagMask = (1 << kWeakReferenceTagSize) - 1; // is tagged with the page header tag, we can just look up the owner.
// Otherwise, we know that we are somewhere (not within the first 1M) in a
// large object.
const int kPageHeaderTag = 3;
const int kPageHeaderTagSize = 2;
const intptr_t kPageHeaderTagMask = (1 << kPageHeaderTagSize) - 1;
// Zap-value: The value used for zapping dead objects. // Zap-value: The value used for zapping dead objects.
// Should be a recognizable hex value tagged as a failure. // Should be a recognizable hex value tagged as a failure.
...@@ -538,6 +544,7 @@ enum AllocationSpace { ...@@ -538,6 +544,7 @@ enum AllocationSpace {
LAST_PAGED_SPACE = MAP_SPACE LAST_PAGED_SPACE = MAP_SPACE
}; };
const int kSpaceTagSize = 3; const int kSpaceTagSize = 3;
const int kSpaceTagMask = (1 << kSpaceTagSize) - 1;
enum AllocationAlignment { kWordAligned, kDoubleAligned, kDoubleUnaligned }; enum AllocationAlignment { kWordAligned, kDoubleAligned, kDoubleUnaligned };
......
...@@ -680,7 +680,7 @@ void IncrementalMarking::RevisitObject(HeapObject* obj) { ...@@ -680,7 +680,7 @@ void IncrementalMarking::RevisitObject(HeapObject* obj) {
DCHECK(IsMarking()); DCHECK(IsMarking());
DCHECK(FLAG_concurrent_marking || marking_state()->IsBlack(obj)); DCHECK(FLAG_concurrent_marking || marking_state()->IsBlack(obj));
Page* page = Page::FromAddress(obj->address()); Page* page = Page::FromAddress(obj->address());
if (page->owner()->identity() == LO_SPACE) { if ((page->owner() != nullptr) && (page->owner()->identity() == LO_SPACE)) {
page->ResetProgressBar(); page->ResetProgressBar();
} }
Map* map = obj->map(); Map* map = obj->map();
......
...@@ -144,13 +144,14 @@ void MemoryChunk::InitializeFreeListCategories() { ...@@ -144,13 +144,14 @@ void MemoryChunk::InitializeFreeListCategories() {
} }
bool PagedSpace::Contains(Address addr) { bool PagedSpace::Contains(Address addr) {
if (heap_->lo_space()->FindPage(addr)) return false;
return MemoryChunk::FromAnyPointerAddress(heap(), addr)->owner() == this; return MemoryChunk::FromAnyPointerAddress(heap(), addr)->owner() == this;
} }
bool PagedSpace::Contains(Object* o) { bool PagedSpace::Contains(Object* o) {
if (!o->IsHeapObject()) return false; if (!o->IsHeapObject()) return false;
return Page::FromAddress(HeapObject::cast(o)->address())->owner() == this; Page* p = Page::FromAddress(HeapObject::cast(o)->address());
if (!Page::IsValid(p)) return false;
return p->owner() == this;
} }
void PagedSpace::UnlinkFreeListCategories(Page* page) { void PagedSpace::UnlinkFreeListCategories(Page* page) {
......
...@@ -3217,6 +3217,7 @@ LargePage* LargeObjectSpace::FindPage(Address a) { ...@@ -3217,6 +3217,7 @@ LargePage* LargeObjectSpace::FindPage(Address a) {
auto it = chunk_map_.find(reinterpret_cast<Address>(key)); auto it = chunk_map_.find(reinterpret_cast<Address>(key));
if (it != chunk_map_.end()) { if (it != chunk_map_.end()) {
LargePage* page = it->second; LargePage* page = it->second;
DCHECK(LargePage::IsValid(page));
if (page->Contains(a)) { if (page->Contains(a)) {
return page; return page;
} }
......
...@@ -423,6 +423,8 @@ class MemoryChunk { ...@@ -423,6 +423,8 @@ class MemoryChunk {
!chunk->high_water_mark_.TrySetValue(old_mark, new_mark)); !chunk->high_water_mark_.TrySetValue(old_mark, new_mark));
} }
static bool IsValid(MemoryChunk* chunk) { return chunk != nullptr; }
Address address() const { Address address() const {
return reinterpret_cast<Address>(const_cast<MemoryChunk*>(this)); return reinterpret_cast<Address>(const_cast<MemoryChunk*>(this));
} }
...@@ -606,9 +608,25 @@ class MemoryChunk { ...@@ -606,9 +608,25 @@ class MemoryChunk {
void set_prev_chunk(MemoryChunk* prev) { prev_chunk_.SetValue(prev); } void set_prev_chunk(MemoryChunk* prev) { prev_chunk_.SetValue(prev); }
Space* owner() const { return owner_; } Space* owner() const {
uintptr_t owner_value = base::AsAtomicWord::Acquire_Load(
reinterpret_cast<const uintptr_t*>(&owner_));
return ((owner_value & kPageHeaderTagMask) == kPageHeaderTag)
? reinterpret_cast<Space*>(owner_value - kPageHeaderTag)
: nullptr;
}
void set_owner(Space* space) {
DCHECK_EQ(0, reinterpret_cast<uintptr_t>(space) & kPageHeaderTagMask);
base::AsAtomicWord::Release_Store(
reinterpret_cast<uintptr_t*>(&owner_),
reinterpret_cast<uintptr_t>(space) + kPageHeaderTag);
DCHECK_EQ(kPageHeaderTag, base::AsAtomicWord::Relaxed_Load(
reinterpret_cast<const uintptr_t*>(&owner_)) &
kPageHeaderTagMask);
}
void set_owner(Space* space) { owner_ = space; } bool HasPageHeader() { return owner() != nullptr; }
void InsertAfter(MemoryChunk* other); void InsertAfter(MemoryChunk* other);
void Unlink(); void Unlink();
...@@ -643,8 +661,10 @@ class MemoryChunk { ...@@ -643,8 +661,10 @@ class MemoryChunk {
// If the chunk needs to remember its memory reservation, it is stored here. // If the chunk needs to remember its memory reservation, it is stored here.
VirtualMemory reservation_; VirtualMemory reservation_;
// The space owning this memory chunk. // The identity of the owning space. This is tagged as a failure pointer, but
Space* owner_; // no failure can be in an object, so this can be distinguished from any entry
// in a fixed array.
Address owner_;
Heap* heap_; Heap* heap_;
...@@ -980,7 +1000,7 @@ class Space : public Malloced { ...@@ -980,7 +1000,7 @@ class Space : public Malloced {
std::vector<AllocationObserver*> allocation_observers_; std::vector<AllocationObserver*> allocation_observers_;
bool allocation_observers_paused_; bool allocation_observers_paused_;
protected: private:
Heap* heap_; Heap* heap_;
AllocationSpace id_; AllocationSpace id_;
Executability executable_; Executability executable_;
......
...@@ -230,6 +230,7 @@ TEST(MemoryAllocator) { ...@@ -230,6 +230,7 @@ TEST(MemoryAllocator) {
NOT_EXECUTABLE); NOT_EXECUTABLE);
first_page->InsertAfter(faked_space.anchor()->prev_page()); first_page->InsertAfter(faked_space.anchor()->prev_page());
CHECK(Page::IsValid(first_page));
CHECK(first_page->next_page() == faked_space.anchor()); CHECK(first_page->next_page() == faked_space.anchor());
total_pages++; total_pages++;
...@@ -241,6 +242,7 @@ TEST(MemoryAllocator) { ...@@ -241,6 +242,7 @@ TEST(MemoryAllocator) {
Page* other = memory_allocator->AllocatePage( Page* other = memory_allocator->AllocatePage(
faked_space.AreaSize(), static_cast<PagedSpace*>(&faked_space), faked_space.AreaSize(), static_cast<PagedSpace*>(&faked_space),
NOT_EXECUTABLE); NOT_EXECUTABLE);
CHECK(Page::IsValid(other));
total_pages++; total_pages++;
other->InsertAfter(first_page); other->InsertAfter(first_page);
int page_count = 0; int page_count = 0;
...@@ -251,7 +253,7 @@ TEST(MemoryAllocator) { ...@@ -251,7 +253,7 @@ TEST(MemoryAllocator) {
CHECK(total_pages == page_count); CHECK(total_pages == page_count);
Page* second_page = first_page->next_page(); Page* second_page = first_page->next_page();
CHECK_NOT_NULL(second_page); CHECK(Page::IsValid(second_page));
// OldSpace's destructor will tear down the space and free up all pages. // OldSpace's destructor will tear down the space and free up all pages.
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment