# HG changeset patch # User iklam # Date 1569293677 25200 # Node ID 00a98f0aa1b3f733183bf2ddc94f75d9d95ee0f7 # Parent 1e57d3774190a452d2a2f466a64bf80541b68296 8231257: Avoid calling FileMapInfo::write_region twice Reviewed-by: ccheung diff -r 1e57d3774190 -r 00a98f0aa1b3 src/hotspot/share/memory/dynamicArchive.cpp --- a/src/hotspot/share/memory/dynamicArchive.cpp Mon Sep 23 13:59:41 2019 -0700 +++ b/src/hotspot/share/memory/dynamicArchive.cpp Mon Sep 23 19:54:37 2019 -0700 @@ -471,6 +471,7 @@ void set_symbols_permanent(); void relocate_buffer_to_target(); void write_archive(char* read_only_tables_start); + void write_regions(FileMapInfo* dynamic_info); void init_first_dump_space(address reserved_bottom) { address first_space_base = reserved_bottom; @@ -911,9 +912,7 @@ _header->relocate_shared_path_table(table); } -static void write_archive_info(FileMapInfo* dynamic_info, DynamicArchiveHeader *header) { - dynamic_info->write_header(); - dynamic_info->align_file_position(); +void DynamicArchiveBuilder::write_regions(FileMapInfo* dynamic_info) { dynamic_info->write_region(MetaspaceShared::rw, MetaspaceShared::read_write_dump_space()->base(), MetaspaceShared::read_write_dump_space()->used(), @@ -937,19 +936,14 @@ FileMapInfo* dynamic_info = FileMapInfo::dynamic_info(); assert(dynamic_info != NULL, "Sanity"); - // Populate the file offsets, region crcs, etc. No data is written out. - write_archive_info(dynamic_info, _header); - - // the header will no longer change. Compute its crc. - dynamic_info->set_header_crc(dynamic_info->compute_header_crc()); - // Now write the archived data including the file offsets. const char* archive_name = Arguments::GetSharedDynamicArchivePath(); dynamic_info->open_for_write(archive_name); - write_archive_info(dynamic_info, _header); + write_regions(dynamic_info); + dynamic_info->set_header_crc(dynamic_info->compute_header_crc()); + dynamic_info->write_header(); dynamic_info->close(); - address base = to_target(_alloc_bottom); address top = address(current_dump_space()->top()) + _buffer_to_target_delta; size_t file_size = pointer_delta(top, base, sizeof(char)); diff -r 1e57d3774190 -r 00a98f0aa1b3 src/hotspot/share/memory/filemap.cpp --- a/src/hotspot/share/memory/filemap.cpp Mon Sep 23 13:59:41 2019 -0700 +++ b/src/hotspot/share/memory/filemap.cpp Mon Sep 23 19:54:37 2019 -0700 @@ -1033,6 +1033,11 @@ return true; } +void FileMapInfo::seek_to_position(size_t pos) { + if (lseek(_fd, (long)pos, SEEK_SET) < 0) { + fail_stop("Unable to seek to position " SIZE_FORMAT, pos); + } +} // Read the FileMapInfo information from the file. bool FileMapInfo::open_for_read(const char* path) { @@ -1088,14 +1093,26 @@ os::strerror(errno)); } _fd = fd; - _file_offset = 0; _file_open = true; + + // Seek past the header. We will write the header after all regions are written + // and their CRCs computed. + size_t header_bytes = header()->header_size(); + if (header()->magic() == CDS_DYNAMIC_ARCHIVE_MAGIC) { + header_bytes += strlen(Arguments::GetSharedArchivePath()) + 1; + } + + header_bytes = align_up(header_bytes, os::vm_allocation_granularity()); + _file_offset = header_bytes; + seek_to_position(_file_offset); } // Write the header to the file, seek to the next allocation boundary. void FileMapInfo::write_header() { + _file_offset = 0; + seek_to_position(_file_offset); char* base_archive_name = NULL; if (header()->magic() == CDS_DYNAMIC_ARCHIVE_MAGIC) { base_archive_name = (char*)Arguments::GetSharedArchivePath(); @@ -1108,7 +1125,6 @@ if (base_archive_name != NULL) { write_bytes(base_archive_name, header()->base_archive_name_size()); } - align_file_position(); } void FileMapRegion::init(bool is_heap_region, char* base, size_t size, bool read_only, @@ -1132,9 +1148,6 @@ _crc = crc; } -// Dump region to file. -// This is called twice for each region during archiving, once before -// the archive file is open (_file_open is false) and once after. void FileMapInfo::write_region(int region, char* base, size_t size, bool read_only, bool allow_exec) { assert(DumpSharedSpaces || DynamicDumpSharedSpaces, "Dump time only"); @@ -1146,14 +1159,10 @@ target_base = DynamicArchive::buffer_to_target(base); } - if (_file_open) { - guarantee(si->file_offset() == _file_offset, "file offset mismatch."); - log_info(cds)("Shared file region %d: " SIZE_FORMAT_HEX_W(08) - " bytes, addr " INTPTR_FORMAT " file offset " SIZE_FORMAT_HEX_W(08), - region, size, p2i(target_base), _file_offset); - } else { - si->set_file_offset(_file_offset); - } + si->set_file_offset(_file_offset); + log_info(cds)("Shared file region %d: " SIZE_FORMAT_HEX_W(08) + " bytes, addr " INTPTR_FORMAT " file offset " SIZE_FORMAT_HEX_W(08), + region, size, p2i(target_base), _file_offset); int crc = ClassLoader::crc32(0, base, (jint)size); si->init(HeapShared::is_heap_region(region), target_base, size, read_only, allow_exec, crc); @@ -1196,8 +1205,7 @@ // +-- gap size_t FileMapInfo::write_archive_heap_regions(GrowableArray *heap_mem, GrowableArray *oopmaps, - int first_region_id, int max_num_regions, - bool print_log) { + int first_region_id, int max_num_regions) { assert(max_num_regions <= 2, "Only support maximum 2 memory regions"); int arr_len = heap_mem == NULL ? 0 : heap_mem->length(); @@ -1221,10 +1229,8 @@ total_size += size; } - if (print_log) { - log_info(cds)("Archive heap region %d " INTPTR_FORMAT " - " INTPTR_FORMAT " = " SIZE_FORMAT_W(8) " bytes", - i, p2i(start), p2i(start + size), size); - } + log_info(cds)("Archive heap region %d " INTPTR_FORMAT " - " INTPTR_FORMAT " = " SIZE_FORMAT_W(8) " bytes", + i, p2i(start), p2i(start + size), size); write_region(i, start, size, false, false); if (size > 0) { space_at(i)->init_oopmap(oopmaps->at(arr_idx)._oopmap, @@ -1237,14 +1243,13 @@ // Dump bytes to file -- at the current file position. void FileMapInfo::write_bytes(const void* buffer, size_t nbytes) { - if (_file_open) { - size_t n = os::write(_fd, buffer, (unsigned int)nbytes); - if (n != nbytes) { - // If the shared archive is corrupted, close it and remove it. - close(); - remove(_full_path); - fail_stop("Unable to write to shared archive file."); - } + assert(_file_open, "must be"); + size_t n = os::write(_fd, buffer, (unsigned int)nbytes); + if (n != nbytes) { + // If the shared archive is corrupted, close it and remove it. + close(); + remove(_full_path); + fail_stop("Unable to write to shared archive file."); } _file_offset += nbytes; } @@ -1257,20 +1262,17 @@ // Align file position to an allocation unit boundary. void FileMapInfo::align_file_position() { + assert(_file_open, "must be"); size_t new_file_offset = align_up(_file_offset, - os::vm_allocation_granularity()); + os::vm_allocation_granularity()); if (new_file_offset != _file_offset) { _file_offset = new_file_offset; - if (_file_open) { - // Seek one byte back from the target and write a byte to insure - // that the written file is the correct length. - _file_offset -= 1; - if (lseek(_fd, (long)_file_offset, SEEK_SET) < 0) { - fail_stop("Unable to seek."); - } - char zero = 0; - write_bytes(&zero, 1); - } + // Seek one byte back from the target and write a byte to insure + // that the written file is the correct length. + _file_offset -= 1; + seek_to_position(_file_offset); + char zero = 0; + write_bytes(&zero, 1); } } diff -r 1e57d3774190 -r 00a98f0aa1b3 src/hotspot/share/memory/filemap.hpp --- a/src/hotspot/share/memory/filemap.hpp Mon Sep 23 13:59:41 2019 -0700 +++ b/src/hotspot/share/memory/filemap.hpp Mon Sep 23 19:54:37 2019 -0700 @@ -412,8 +412,7 @@ bool read_only, bool allow_exec); size_t write_archive_heap_regions(GrowableArray *heap_mem, GrowableArray *oopmaps, - int first_region_id, int max_num_regions, - bool print_log); + int first_region_id, int max_num_regions); void write_bytes(const void* buffer, size_t count); void write_bytes_aligned(const void* buffer, size_t count); size_t read_bytes(void* buffer, size_t count); @@ -479,6 +478,7 @@ char* region_addr(int idx); private: + void seek_to_position(size_t pos); char* skip_first_path_entry(const char* path) NOT_CDS_RETURN_(NULL); int num_paths(const char* path) NOT_CDS_RETURN_(0); GrowableArray* create_path_array(const char* path) NOT_CDS_RETURN_(NULL); diff -r 1e57d3774190 -r 00a98f0aa1b3 src/hotspot/share/memory/metaspaceShared.cpp --- a/src/hotspot/share/memory/metaspaceShared.cpp Mon Sep 23 13:59:41 2019 -0700 +++ b/src/hotspot/share/memory/metaspaceShared.cpp Mon Sep 23 19:54:37 2019 -0700 @@ -1137,7 +1137,9 @@ VMOp_Type type() const { return VMOp_PopulateDumpSharedSpace; } void doit(); // outline because gdb sucks - static void write_region(FileMapInfo* mapinfo, int region, DumpRegion* space, bool read_only, bool allow_exec); + static void write_region(FileMapInfo* mapinfo, int region_idx, DumpRegion* dump_region, bool read_only, bool allow_exec) { + mapinfo->write_region(region_idx, dump_region->base(), dump_region->used(), read_only, allow_exec); + } bool allow_nested_vm_operations() const { return true; } }; // class VM_PopulateDumpSharedSpace @@ -1406,11 +1408,6 @@ SortedSymbolClosure* ArchiveCompactor::_ssc; ArchiveCompactor::RelocationTable* ArchiveCompactor::_new_loc_table; -void VM_PopulateDumpSharedSpace::write_region(FileMapInfo* mapinfo, int region_idx, - DumpRegion* dump_region, bool read_only, bool allow_exec) { - mapinfo->write_region(region_idx, dump_region->base(), dump_region->used(), read_only, allow_exec); -} - void VM_PopulateDumpSharedSpace::dump_symbols() { tty->print_cr("Dumping symbol table ..."); @@ -1546,44 +1543,30 @@ mapinfo->set_read_only_tables_start(read_only_tables_start); mapinfo->set_misc_data_patching_start(vtbl_list); mapinfo->set_i2i_entry_code_buffers(MetaspaceShared::i2i_entry_code_buffers(), - MetaspaceShared::i2i_entry_code_buffers_size()); + MetaspaceShared::i2i_entry_code_buffers_size()); mapinfo->set_core_spaces_size(core_spaces_size); + mapinfo->open_for_write(); - for (int pass=1; pass<=2; pass++) { - bool print_archive_log = (pass==1); - if (pass == 1) { - // The first pass doesn't actually write the data to disk. All it - // does is to update the fields in the mapinfo->_header. - } else { - // After the first pass, the contents of mapinfo->_header are finalized, - // so we can compute the header's CRC, and write the contents of the header - // and the regions into disk. - mapinfo->open_for_write(); - mapinfo->set_header_crc(mapinfo->compute_header_crc()); - } - mapinfo->write_header(); + // NOTE: md contains the trampoline code for method entries, which are patched at run time, + // so it needs to be read/write. + write_region(mapinfo, MetaspaceShared::mc, &_mc_region, /*read_only=*/false,/*allow_exec=*/true); + write_region(mapinfo, MetaspaceShared::rw, &_rw_region, /*read_only=*/false,/*allow_exec=*/false); + write_region(mapinfo, MetaspaceShared::ro, &_ro_region, /*read_only=*/true, /*allow_exec=*/false); + write_region(mapinfo, MetaspaceShared::md, &_md_region, /*read_only=*/false,/*allow_exec=*/false); - // NOTE: md contains the trampoline code for method entries, which are patched at run time, - // so it needs to be read/write. - write_region(mapinfo, MetaspaceShared::mc, &_mc_region, /*read_only=*/false,/*allow_exec=*/true); - write_region(mapinfo, MetaspaceShared::rw, &_rw_region, /*read_only=*/false,/*allow_exec=*/false); - write_region(mapinfo, MetaspaceShared::ro, &_ro_region, /*read_only=*/true, /*allow_exec=*/false); - write_region(mapinfo, MetaspaceShared::md, &_md_region, /*read_only=*/false,/*allow_exec=*/false); - - _total_closed_archive_region_size = mapinfo->write_archive_heap_regions( + _total_closed_archive_region_size = mapinfo->write_archive_heap_regions( _closed_archive_heap_regions, _closed_archive_heap_oopmaps, MetaspaceShared::first_closed_archive_heap_region, - MetaspaceShared::max_closed_archive_heap_region, - print_archive_log); - _total_open_archive_region_size = mapinfo->write_archive_heap_regions( + MetaspaceShared::max_closed_archive_heap_region); + _total_open_archive_region_size = mapinfo->write_archive_heap_regions( _open_archive_heap_regions, _open_archive_heap_oopmaps, MetaspaceShared::first_open_archive_heap_region, - MetaspaceShared::max_open_archive_heap_region, - print_archive_log); - } + MetaspaceShared::max_open_archive_heap_region); + mapinfo->set_header_crc(mapinfo->compute_header_crc()); + mapinfo->write_header(); mapinfo->close(); // Restore the vtable in case we invoke any virtual methods.