Skip to content

Commit

Permalink
Bug 1558365 - Simplify PtrInfoTag. r=glandium
Browse files Browse the repository at this point in the history
This makes it less mozjemalloc-specific, which is helpful for PHC. No non-test
code uses the extra detail anyway.

Differential Revision: https://phabricator.services.mozilla.com/D34441

--HG--
extra : moz-landing-system : lando
  • Loading branch information
nnethercote committed Jun 12, 2019
1 parent ed9ef40 commit 6374569
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 44 deletions.
21 changes: 4 additions & 17 deletions memory/build/mozjemalloc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3077,7 +3077,7 @@ inline void MozJemalloc::jemalloc_ptr_info(const void* aPtr,
&huge)
->Search(&key);
if (node) {
*aInfo = {TagLiveHuge, node->mAddr, node->mSize, node->mArena->mId};
*aInfo = {TagLiveAlloc, node->mAddr, node->mSize, node->mArena->mId};
return;
}
}
Expand All @@ -3101,21 +3101,8 @@ inline void MozJemalloc::jemalloc_ptr_info(const void* aPtr,
size_t mapbits = chunk->map[pageind].bits;

if (!(mapbits & CHUNK_MAP_ALLOCATED)) {
PtrInfoTag tag = TagFreedPageDirty;
if (mapbits & CHUNK_MAP_DIRTY) {
tag = TagFreedPageDirty;
} else if (mapbits & CHUNK_MAP_DECOMMITTED) {
tag = TagFreedPageDecommitted;
} else if (mapbits & CHUNK_MAP_MADVISED) {
tag = TagFreedPageMadvised;
} else if (mapbits & CHUNK_MAP_ZEROED) {
tag = TagFreedPageZeroed;
} else {
MOZ_CRASH();
}

void* pageaddr = (void*)(uintptr_t(aPtr) & ~gPageSizeMask);
*aInfo = {tag, pageaddr, gPageSize, chunk->arena->mId};
*aInfo = {TagFreedPage, pageaddr, gPageSize, chunk->arena->mId};
return;
}

Expand Down Expand Up @@ -3148,7 +3135,7 @@ inline void MozJemalloc::jemalloc_ptr_info(const void* aPtr,
}

void* addr = ((char*)chunk) + (pageind << gPageSize2Pow);
*aInfo = {TagLiveLarge, addr, size, chunk->arena->mId};
*aInfo = {TagLiveAlloc, addr, size, chunk->arena->mId};
return;
}

Expand Down Expand Up @@ -3177,7 +3164,7 @@ inline void MozJemalloc::jemalloc_ptr_info(const void* aPtr,
unsigned elm = regind >> (LOG2(sizeof(int)) + 3);
unsigned bit = regind - (elm << (LOG2(sizeof(int)) + 3));
PtrInfoTag tag =
((run->mRegionsMask[elm] & (1U << bit))) ? TagFreedSmall : TagLiveSmall;
((run->mRegionsMask[elm] & (1U << bit))) ? TagFreedAlloc : TagLiveAlloc;

*aInfo = {tag, addr, size, chunk->arena->mId};
}
Expand Down
22 changes: 6 additions & 16 deletions memory/build/mozjemalloc_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,21 +99,16 @@ enum PtrInfoTag {

// The pointer is within a live allocation.
// 'addr', 'size', and 'arenaId' describe the allocation.
TagLiveSmall,
TagLiveLarge,
TagLiveHuge,
TagLiveAlloc,

// The pointer is within a small freed allocation.
// 'addr', 'size', and 'arenaId' describe the allocation.
TagFreedSmall,
TagFreedAlloc,

// The pointer is within a freed page. Details about the original
// allocation, including its size, are not available.
// 'addr', 'size', and 'arenaId' describe the page.
TagFreedPageDirty,
TagFreedPageDecommitted,
TagFreedPageMadvised,
TagFreedPageZeroed,
TagFreedPage,
};

// The information in jemalloc_ptr_info_t could be represented in a variety of
Expand Down Expand Up @@ -147,20 +142,15 @@ typedef struct jemalloc_ptr_info_s {
} jemalloc_ptr_info_t;

static inline bool jemalloc_ptr_is_live(jemalloc_ptr_info_t* info) {
return info->tag == TagLiveSmall || info->tag == TagLiveLarge ||
info->tag == TagLiveHuge;
return info->tag == TagLiveAlloc;
}

static inline bool jemalloc_ptr_is_freed(jemalloc_ptr_info_t* info) {
return info->tag == TagFreedSmall || info->tag == TagFreedPageDirty ||
info->tag == TagFreedPageDecommitted ||
info->tag == TagFreedPageMadvised || info->tag == TagFreedPageZeroed;
return info->tag == TagFreedAlloc || info->tag == TagFreedPage;
}

static inline bool jemalloc_ptr_is_freed_page(jemalloc_ptr_info_t* info) {
return info->tag == TagFreedPageDirty ||
info->tag == TagFreedPageDecommitted ||
info->tag == TagFreedPageMadvised || info->tag == TagFreedPageZeroed;
return info->tag == TagFreedPage;
}

#ifdef __cplusplus
Expand Down
22 changes: 11 additions & 11 deletions memory/gtest/TestJemalloc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ TEST(Jemalloc, PtrInfo)
ASSERT_TRUE(small.append(p));
for (size_t j = 0; j < usable; j++) {
jemalloc_ptr_info(&p[j], &info);
ASSERT_TRUE(InfoEq(info, TagLiveSmall, p, usable, arenaId));
ASSERT_TRUE(InfoEq(info, TagLiveAlloc, p, usable, arenaId));
}
}

Expand All @@ -137,7 +137,7 @@ TEST(Jemalloc, PtrInfo)
ASSERT_TRUE(large.append(p));
for (size_t j = 0; j < usable; j += 347) {
jemalloc_ptr_info(&p[j], &info);
ASSERT_TRUE(InfoEq(info, TagLiveLarge, p, usable, arenaId));
ASSERT_TRUE(InfoEq(info, TagLiveAlloc, p, usable, arenaId));
}
}

Expand All @@ -148,7 +148,7 @@ TEST(Jemalloc, PtrInfo)
ASSERT_TRUE(huge.append(p));
for (size_t j = 0; j < usable; j += 567) {
jemalloc_ptr_info(&p[j], &info);
ASSERT_TRUE(InfoEq(info, TagLiveHuge, p, usable, arenaId));
ASSERT_TRUE(InfoEq(info, TagLiveAlloc, p, usable, arenaId));
}
}

Expand All @@ -158,7 +158,7 @@ TEST(Jemalloc, PtrInfo)
size_t len;

// Free the small allocations and recheck them.
int isFreedSmall = 0, isFreedPage = 0;
int isFreedAlloc = 0, isFreedPage = 0;
len = small.length();
for (size_t i = 0, j = 0; i < len; i++, j = (j + 19) % len) {
char* p = small[j];
Expand All @@ -167,20 +167,20 @@ TEST(Jemalloc, PtrInfo)
for (size_t k = 0; k < usable; k++) {
jemalloc_ptr_info(&p[k], &info);
// There are two valid outcomes here.
if (InfoEq(info, TagFreedSmall, p, usable, arenaId)) {
isFreedSmall++;
if (InfoEq(info, TagFreedAlloc, p, usable, arenaId)) {
isFreedAlloc++;
} else if (InfoEqFreedPage(info, &p[k], stats.page_size, arenaId)) {
isFreedPage++;
} else {
ASSERT_TRUE(false);
}
}
}
// There should be both FreedSmall and FreedPage results, but a lot more of
// There should be both FreedAlloc and FreedPage results, but a lot more of
// the former.
ASSERT_TRUE(isFreedSmall != 0);
ASSERT_TRUE(isFreedAlloc != 0);
ASSERT_TRUE(isFreedPage != 0);
ASSERT_TRUE(isFreedSmall / isFreedPage > 10);
ASSERT_TRUE(isFreedAlloc / isFreedPage > 10);

// Free the large allocations and recheck them.
len = large.length();
Expand Down Expand Up @@ -660,7 +660,7 @@ TEST(Jemalloc, GuardRegion)
jemalloc_ptr_info_t info;
jemalloc_ptr_info(guard_page, &info);
ASSERT_TRUE(jemalloc_ptr_is_freed_page(&info));
ASSERT_TRUE(info.tag == TagFreedPageDecommitted);
ASSERT_TRUE(info.tag == TagFreedPage);

ASSERT_DEATH_WRAP(*(char*)guard_page = 0, "");

Expand All @@ -677,4 +677,4 @@ TEST(Jemalloc, GuardRegion)
# endif
}

#endif
#endif

0 comments on commit 6374569

Please sign in to comment.