Skip to content

Commit

Permalink
Optimize access to gPageSize
Browse files Browse the repository at this point in the history
This change refactors the global constants dependent on gPageSize in
a way that gPageSize and dependent constants are now internal to the
runtime. Also it removes static initialization dependency on gPageSize
and disallows accessing gPageSize during the static initialization
phase. Consequently, this removes a need in the condition check when
accessing gPageSize and allows compiler to recognise the value as
a constant that can be cached where used repeatedly.

From this point, gPageSize should only be used within libart. For most
of the other cases MemMap::GetPageSize() should be used. Where MemMap
is unavailable e.g. during static initialization or another stage when
MemMap isn't yet initialized, or in a component which might operate
without MemMap being initialized, the GetPageSizeSlow() would be
generally suitable. For performance-sensitive code, GetPageSizeSlow()
shouldn't be used without caching the value to remove repeated calls
of the function.

-  The gPageSize constant is now local to libart. Access to the variable
   is disallowed until Runtime is initialized, so removing the chance of
   static initialization order issue related to this value.

   The initializers for the other constants with initialization
   that used to depend on the gPageSize value are now referring to
   GetPageSizeSlow instead, so removing the static initialization
   dependency, and consequently, possible order issues.

   This removes the condition check upon each load of the gPageSize.

   Also as the variable is defined with "hidden" visibility, it isn't
   exported and so is unavailable to other DSOs.

   This also removes one indirection level in accessing it as GOT lookup
   is not required anymore.

 - Remove gPMDSize and gPUDSize from libartbase

   Move the constants to gc::Heap in form of functions computing their
   values. Also move the BestPageTableAlignment function dependent on
   the values, as it is only used within the GC.

   Direct accesses to gPMDSize are only performed for assertions and so
   replacing that with a function call shouldn't matter for performance.

   gPUDSize isn't accessed directly.

   The rest the values accesses are done via BestPageTableAlignment,
   which is called from:
    - MarkCompact constructor
    - MarkCompact::AddLinearAllocSpaceData
    - GcVisitedArenaPool::AddMap

   In all cases, it's performed in conjunction with MapAnonymousAligned,
   which involves an mmap system call. The potential slight slowdown
   in BestPageTableAlignment - due to the dynamic computation of the
   gPMDSize and gPUDSize values - is expected to be negligible in
   comparison to the performance cost of mmap system call.

 - Move gPageSize declaration and definition to the runtime / component

   Replace the removed uses of gPageSize outside libart with
   MemMap::GetPageSize() / GetPageSizeSlow().

   There is one exception to the above statement in this change - in
   FdFile, where the page size appears to be used arbitrarily to compute
   the maximum buffer size: in that case replace the maximum buffer size
   with a constant which would have matched the value when the page size
   is 4K.

 - Make gNumLrtSlots a member variable

   The gNumLrtSlots global is only used within the SmallLrtAllocator
   class. Move it inside the class as a member variable.

 - Remove gStackOverflowProtectedSize global

   Value of the gStackOverflowProtectedSize is computed as a page size
   multiplied by a constant dependent on whether the address sanitizer
   is enabled. If address sanitizer is disabled, the constant is 1,
   in which case gStackOverflowProtectedSize is equal to the page size.

   Replace the global with an always-inlined function which is expected
   to expand into a simple reference to the gPageSize at each call site
   if the address sanitizer is not enabled.

 - Remove ART_PAGE_SIZE_AGNOSTIC_DECLARE_* macros
   The macros aren't used anymore. Remove them and GlobalConst.

The tests were run for legacy 4K, page size agnostic 4K and 16K.

Co-authored-by: Ruben Ayrapetyan <[email protected]>
Co-authored-by: Richard Neill <[email protected]>
Test: art/tools/run-gtests.sh
Test: art/test/testrunner/testrunner.py --target --64
Test: art/tools/run-libcore-tests.sh --mode=device --variant=X64
Test: art/tools/run-libjdwp-tests.sh --mode=device --variant=X64
Test: cd art && mma
Change-Id: I07fd85cd35dd443dee5cff0f7435ab47ba727c6f
  • Loading branch information
2 people authored and Treehugger Robot committed Dec 27, 2023
1 parent 225de25 commit b343f66
Show file tree
Hide file tree
Showing 41 changed files with 528 additions and 395 deletions.
5 changes: 3 additions & 2 deletions compiler/common_compiler_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,16 @@ class CommonCompilerTestImpl::CodeAndMetadata {
CodeAndMetadata(ArrayRef<const uint8_t> code,
ArrayRef<const uint8_t> vmap_table,
InstructionSet instruction_set) {
const size_t page_size = MemMap::GetPageSize();
const uint32_t code_size = code.size();
CHECK_NE(code_size, 0u);
const uint32_t vmap_table_offset = vmap_table.empty() ? 0u
: sizeof(OatQuickMethodHeader) + vmap_table.size();
OatQuickMethodHeader method_header(vmap_table_offset);
const size_t code_alignment = GetInstructionSetCodeAlignment(instruction_set);
DCHECK_ALIGNED_PARAM(static_cast<size_t>(gPageSize), code_alignment);
DCHECK_ALIGNED_PARAM(page_size, code_alignment);
const uint32_t code_offset = RoundUp(vmap_table.size() + sizeof(method_header), code_alignment);
const uint32_t capacity = RoundUp(code_offset + code_size, gPageSize);
const uint32_t capacity = RoundUp(code_offset + code_size, page_size);

// Create a memfd handle with sufficient capacity.
android::base::unique_fd mem_fd(art::memfd_create_compat("test code", /*flags=*/ 0));
Expand Down
4 changes: 2 additions & 2 deletions compiler/jit/jit_logger.cc
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ void JitLogger::OpenMarkerFile() {
int fd = jit_dump_file_->Fd();
// The 'perf inject' tool requires that the jit-PID.dump file
// must have a mmap(PROT_READ|PROT_EXEC) record in perf.data.
marker_address_ = mmap(nullptr, gPageSize, PROT_READ | PROT_EXEC, MAP_PRIVATE, fd, 0);
marker_address_ = mmap(nullptr, MemMap::GetPageSize(), PROT_READ | PROT_EXEC, MAP_PRIVATE, fd, 0);
if (marker_address_ == MAP_FAILED) {
LOG(WARNING) << "Failed to create record in perf.data. JITed code profiling will not work.";
return;
Expand All @@ -220,7 +220,7 @@ void JitLogger::OpenMarkerFile() {

void JitLogger::CloseMarkerFile() {
if (marker_address_ != nullptr) {
munmap(marker_address_, gPageSize);
munmap(marker_address_, MemMap::GetPageSize());
}
}

Expand Down
5 changes: 3 additions & 2 deletions dex2oat/common_compiler_driver_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,14 @@ void CommonCompilerDriverTest::SetUp() {

// Note: We cannot use MemMap because some tests tear down the Runtime and destroy
// the gMaps, so when destroying the MemMap, the test would crash.
inaccessible_page_ = mmap(nullptr, gPageSize, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
const size_t page_size = MemMap::GetPageSize();
inaccessible_page_ = mmap(nullptr, page_size, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
CHECK(inaccessible_page_ != MAP_FAILED) << strerror(errno);
}

void CommonCompilerDriverTest::TearDown() {
if (inaccessible_page_ != nullptr) {
munmap(inaccessible_page_, gPageSize);
munmap(inaccessible_page_, MemMap::GetPageSize());
inaccessible_page_ = nullptr;
}
image_reservation_.Reset();
Expand Down
2 changes: 1 addition & 1 deletion dex2oat/linker/elf_writer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ TEST_F(ElfWriterTest, dlsym) {
bool success = ef->GetLoadedSize(&size, &error_msg);
CHECK(success) << error_msg;
MemMap reservation = MemMap::MapAnonymous("ElfWriterTest#dlsym reservation",
RoundUp(size, gPageSize),
RoundUp(size, MemMap::GetPageSize()),
PROT_NONE,
/*low_4gb=*/ true,
&error_msg);
Expand Down
4 changes: 3 additions & 1 deletion dex2oat/utils/swap_space.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <numeric>

#include "base/bit_utils.h"
#include "base/mem_map.h"
#include "base/macros.h"
#include "base/mutex.h"
#include "thread-current-inl.h"
Expand Down Expand Up @@ -146,7 +147,8 @@ void* SwapSpace::Alloc(size_t size) {

SwapSpace::SpaceChunk SwapSpace::NewFileChunk(size_t min_size) {
#if !defined(__APPLE__)
size_t next_part = std::max(RoundUp(min_size, gPageSize), RoundUp(kMinimumMapSize, gPageSize));
const size_t page_size = MemMap::GetPageSize();
size_t next_part = std::max(RoundUp(min_size, page_size), RoundUp(kMinimumMapSize, page_size));
int result = TEMP_FAILURE_RETRY(ftruncate64(fd_, size_ + next_part));
if (result != 0) {
PLOG(FATAL) << "Unable to increase swap file.";
Expand Down
15 changes: 9 additions & 6 deletions dexlayout/dex_visualize.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

#include <android-base/logging.h>

#include "base/mem_map.h"
#include "dex_ir.h"
#include "dexlayout.h"
#include "profile/profile_compilation_info.h"
Expand All @@ -49,7 +50,8 @@ class Dumper {
explicit Dumper(dex_ir::Header* header)
: out_file_(nullptr),
sorted_sections_(
dex_ir::GetSortedDexFileSections(header, dex_ir::SortDirection::kSortDescending)) { }
dex_ir::GetSortedDexFileSections(header, dex_ir::SortDirection::kSortDescending)),
page_size_(MemMap::GetPageSize()) {}

bool OpenAndPrintHeader(size_t dex_index) {
// Open the file and emit the gnuplot prologue.
Expand All @@ -70,7 +72,7 @@ class Dumper {
if (printed_one) {
fprintf(out_file_, ", ");
}
fprintf(out_file_, "\"%s\" %" PRIuPTR, s.name.c_str(), s.offset / gPageSize);
fprintf(out_file_, "\"%s\" %" PRIuPTR, s.name.c_str(), s.offset / page_size_);
printed_one = true;
}
}
Expand Down Expand Up @@ -98,8 +100,8 @@ class Dumper {
}

void DumpAddressRange(uint32_t from, uint32_t size, int class_index) {
const uint32_t low_page = from / gPageSize;
const uint32_t high_page = (size > 0) ? (from + size - 1) / gPageSize : low_page;
const uint32_t low_page = from / page_size_;
const uint32_t high_page = (size > 0) ? (from + size - 1) / page_size_ : low_page;
const uint32_t size_delta = high_page - low_page;
fprintf(out_file_, "%d %d %d 0 %d\n", low_page, class_index, size_delta, GetColor(from));
}
Expand Down Expand Up @@ -234,6 +236,7 @@ class Dumper {

FILE* out_file_;
std::vector<dex_ir::DexFileSection> sorted_sections_;
const size_t page_size_;

DISALLOW_COPY_AND_ASSIGN(Dumper);
};
Expand Down Expand Up @@ -316,7 +319,7 @@ static uint32_t FindNextByteAfterSection(dex_ir::Header* header,
/*
* Dumps the offset and size of sections within the file.
*/
void ShowDexSectionStatistics(dex_ir::Header* header, size_t dex_file_index) {
void ShowDexSectionStatistics(dex_ir::Header* header, size_t dex_file_index, size_t page_size) {
// Compute the (multidex) class file name).
fprintf(stdout, "%s (%d bytes)\n",
MultidexName("classes", dex_file_index, ".dex").c_str(),
Expand All @@ -336,7 +339,7 @@ void ShowDexSectionStatistics(dex_ir::Header* header, size_t dex_file_index) {
file_section.offset,
file_section.size,
bytes,
RoundUp(bytes, gPageSize) / gPageSize,
RoundUp(bytes, page_size) / page_size,
100 * bytes / header->FileSize());
}
fprintf(stdout, "\n");
Expand Down
2 changes: 1 addition & 1 deletion dexlayout/dex_visualize.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ void VisualizeDexLayout(dex_ir::Header* header,
size_t dex_file_index,
ProfileCompilationInfo* profile_info);

void ShowDexSectionStatistics(dex_ir::Header* header, size_t dex_file_index);
void ShowDexSectionStatistics(dex_ir::Header* header, size_t dex_file_index, size_t page_size);

} // namespace art

Expand Down
13 changes: 7 additions & 6 deletions dexlayout/dexdiag.cc
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ static char PageTypeChar(uint16_t type) {
static uint16_t FindSectionTypeForPage(size_t page,
const std::vector<dex_ir::DexFileSection>& sections) {
for (const auto& section : sections) {
size_t first_page_of_section = section.offset / gPageSize;
size_t first_page_of_section = section.offset / MemMap::GetPageSize();
// Only consider non-empty sections.
if (section.size == 0) {
continue;
Expand Down Expand Up @@ -287,14 +287,15 @@ static void ProcessOneDexMapping(const std::vector<uint64_t>& pagemap,
<< std::endl;
return;
}
uint64_t start_page = (dex_file_start - vdex_start) / gPageSize;
uint64_t start_address = start_page * gPageSize;
uint64_t end_page = RoundUp(start_address + dex_file_size, gPageSize) / gPageSize;
uint64_t start_page = (dex_file_start - vdex_start) / MemMap::GetPageSize();
uint64_t start_address = start_page * MemMap::GetPageSize();
uint64_t end_page = RoundUp(start_address + dex_file_size,
MemMap::GetPageSize()) / MemMap::GetPageSize();
std::cout << "DEX "
<< dex_file->GetLocation().c_str()
<< StringPrintf(": %" PRIx64 "-%" PRIx64,
map_start + start_page * gPageSize,
map_start + end_page * gPageSize)
map_start + start_page * MemMap::GetPageSize(),
map_start + end_page * MemMap::GetPageSize())
<< std::endl;
// Build a list of the dex file section types, sorted from highest offset to lowest.
std::vector<dex_ir::DexFileSection> sections;
Expand Down
2 changes: 1 addition & 1 deletion dexlayout/dexlayout.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2218,7 +2218,7 @@ bool DexLayout::ProcessDexFile(const char* file_name,
}

if (options_.show_section_statistics_) {
ShowDexSectionStatistics(header_, dex_file_index);
ShowDexSectionStatistics(header_, dex_file_index, page_size_);
return true;
}

Expand Down
5 changes: 4 additions & 1 deletion dexlayout/dexlayout.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <set>
#include <unordered_map>

#include "base/mem_map.h"
#include "dex/compact_dex_level.h"
#include "dex_container.h"
#include "dex/dex_file_layout.h"
Expand Down Expand Up @@ -111,7 +112,8 @@ class DexLayout {
: options_(options),
info_(info),
out_file_(out_file),
header_(header) { }
header_(header),
page_size_(MemMap::GetPageSize()) { }

int ProcessFile(const char* file_name);
bool ProcessDexFile(const char* file_name,
Expand Down Expand Up @@ -194,6 +196,7 @@ class DexLayout {
ProfileCompilationInfo* info_;
FILE* out_file_;
dex_ir::Header* header_;
const size_t page_size_;
DexLayoutSections dex_sections_;
// Layout hotness information is only calculated when dexlayout is enabled.
DexLayoutHotnessInfo layout_hotness_info_;
Expand Down
37 changes: 20 additions & 17 deletions imgdiag/imgdiag.cc
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,9 @@ size_t EntrySize(ArtMethod* art_method) REQUIRES_SHARED(Locks::mutator_lock_) {
// Print all pages the entry belongs to
void PrintEntryPages(uintptr_t entry_address, size_t entry_size, std::ostream& os) {
const char* tabs = " ";
const uintptr_t first_page_idx = entry_address / gPageSize;
const uintptr_t first_page_idx = entry_address / MemMap::GetPageSize();
const uintptr_t last_page_idx = RoundUp(entry_address + entry_size,
kObjectAlignment) / gPageSize;
kObjectAlignment) / MemMap::GetPageSize();
for (uintptr_t page_idx = first_page_idx; page_idx <= last_page_idx; ++page_idx) {
os << tabs << "page_idx=" << page_idx << "\n";
}
Expand Down Expand Up @@ -298,13 +298,14 @@ struct RegionCommon {
uintptr_t entry_address = reinterpret_cast<uintptr_t>(entry);
// Iterate every page this entry belongs to
do {
current_page_idx = entry_address / gPageSize + page_off;
current_page_idx = entry_address / MemMap::GetPageSize() + page_off;
if (dirty_pages.find(current_page_idx) != dirty_pages.end()) {
// This entry is on a dirty page
return true;
}
page_off++;
} while ((current_page_idx * gPageSize) < RoundUp(entry_address + size, kObjectAlignment));
} while ((current_page_idx * MemMap::GetPageSize()) < RoundUp(entry_address + size,
kObjectAlignment));
return false;
}

Expand Down Expand Up @@ -1155,7 +1156,8 @@ class RegionData : public RegionSpecializedBase<T> {
// Looking at only dirty pages, figure out how many of those bytes belong to dirty entries.
// TODO: fix this now that there are multiple regions in a mapping.
float true_dirtied_percent =
RegionCommon<T>::GetDirtyEntryBytes() * 1.0f / (mapping_data.dirty_pages * gPageSize);
(RegionCommon<T>::GetDirtyEntryBytes() * 1.0f) /
(mapping_data.dirty_pages * MemMap::GetPageSize());

// Entry specific statistics.
os_ << RegionCommon<T>::GetDifferentEntryCount() << " different entries, \n "
Expand Down Expand Up @@ -1413,7 +1415,7 @@ class ImgDiagDumper {
MappingData* mapping_data /*out*/,
std::string* error_msg /*out*/) {
// Iterate through one page at a time. Boot map begin/end already implicitly aligned.
for (uintptr_t begin = boot_map.start; begin != boot_map.end; begin += gPageSize) {
for (uintptr_t begin = boot_map.start; begin != boot_map.end; begin += MemMap::GetPageSize()) {
const ptrdiff_t offset = begin - boot_map.start;

// We treat the image header as part of the memory map for now
Expand All @@ -1422,11 +1424,11 @@ class ImgDiagDumper {
const uint8_t* zygote_ptr = &zygote_contents[offset];
const uint8_t* remote_ptr = &remote_contents[offset];

if (memcmp(zygote_ptr, remote_ptr, gPageSize) != 0) {
if (memcmp(zygote_ptr, remote_ptr, MemMap::GetPageSize()) != 0) {
mapping_data->different_pages++;

// Count the number of 32-bit integers that are different.
for (size_t i = 0; i < gPageSize / sizeof(uint32_t); ++i) {
for (size_t i = 0; i < MemMap::GetPageSize() / sizeof(uint32_t); ++i) {
const uint32_t* remote_ptr_int32 = reinterpret_cast<const uint32_t*>(remote_ptr);
const uint32_t* zygote_ptr_int32 = reinterpret_cast<const uint32_t*>(zygote_ptr);

Expand All @@ -1435,19 +1437,19 @@ class ImgDiagDumper {
}
}
// Count the number of bytes that are different.
for (size_t i = 0; i < gPageSize; ++i) {
for (size_t i = 0; i < MemMap::GetPageSize(); ++i) {
if (remote_ptr[i] != zygote_ptr[i]) {
mapping_data->different_bytes++;
}
}
}
}

for (uintptr_t begin = boot_map.start; begin != boot_map.end; begin += gPageSize) {
for (uintptr_t begin = boot_map.start; begin != boot_map.end; begin += MemMap::GetPageSize()) {
ptrdiff_t offset = begin - boot_map.start;

// Virtual page number (for an absolute memory address)
size_t virtual_page_idx = begin / gPageSize;
size_t virtual_page_idx = begin / MemMap::GetPageSize();

uint64_t page_count = 0xC0FFEE;
// TODO: virtual_page_idx needs to be from the same process
Expand Down Expand Up @@ -1555,7 +1557,7 @@ class ImgDiagDumper {

// Adjust the `end` of the mapping. Some other mappings may have been
// inserted within the image.
boot_map.end = RoundUp(boot_map.start + image_header.GetImageSize(), gPageSize);
boot_map.end = RoundUp(boot_map.start + image_header.GetImageSize(), MemMap::GetPageSize());
// The size of the boot image mapping.
size_t boot_map_size = boot_map.end - boot_map.start;

Expand All @@ -1569,7 +1571,8 @@ class ImgDiagDumper {
android::procinfo::MapInfo& zygote_boot_map = *maybe_zygote_boot_map;
// Adjust the `end` of the mapping. Some other mappings may have been
// inserted within the image.
zygote_boot_map.end = RoundUp(zygote_boot_map.start + image_header.GetImageSize(), gPageSize);
zygote_boot_map.end = RoundUp(zygote_boot_map.start + image_header.GetImageSize(),
MemMap::GetPageSize());
if (zygote_boot_map.start != boot_map.start) {
os << "Zygote boot map does not match image boot map: "
<< "zygote begin " << reinterpret_cast<const void*>(zygote_boot_map.start)
Expand All @@ -1589,8 +1592,8 @@ class ImgDiagDumper {
const uint8_t* image_end_unaligned = image_begin_unaligned + image_header.GetImageSize();

// Adjust range to nearest page
const uint8_t* image_begin = AlignDown(image_begin_unaligned, gPageSize);
const uint8_t* image_end = AlignUp(image_end_unaligned, gPageSize);
const uint8_t* image_begin = AlignDown(image_begin_unaligned, MemMap::GetPageSize());
const uint8_t* image_end = AlignUp(image_end_unaligned, MemMap::GetPageSize());

size_t image_size = image_end - image_begin;
if (image_size != boot_map_size) {
Expand All @@ -1603,8 +1606,8 @@ class ImgDiagDumper {
auto read_contents = [&](File* mem_file,
/*out*/ MemMap* map,
/*out*/ ArrayRef<uint8_t>* contents) {
DCHECK_ALIGNED_PARAM(boot_map.start, gPageSize);
DCHECK_ALIGNED_PARAM(boot_map_size, gPageSize);
DCHECK_ALIGNED_PARAM(boot_map.start, MemMap::GetPageSize());
DCHECK_ALIGNED_PARAM(boot_map_size, MemMap::GetPageSize());
std::string name = "Contents of " + mem_file->GetPath();
std::string local_error_msg;
// We need to use low 4 GiB memory so that we can walk the objects using standard
Expand Down
Loading

0 comments on commit b343f66

Please sign in to comment.