From 2fe2f3aff82f41a3b7942861e29ccbd3bcc68661 Mon Sep 17 00:00:00 2001 From: Ioi Lam Date: Wed, 17 Apr 2024 05:31:39 +0000 Subject: [PATCH] 8323900: Avoid calling os::init_random() in CDS static dump Reviewed-by: stuefe, ccheung --- src/hotspot/share/cds/archiveBuilder.cpp | 13 ++++++++++++- src/hotspot/share/cds/archiveBuilder.hpp | 2 ++ src/hotspot/share/cds/metaspaceShared.cpp | 3 --- src/hotspot/share/oops/symbol.cpp | 8 +++----- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/hotspot/share/cds/archiveBuilder.cpp b/src/hotspot/share/cds/archiveBuilder.cpp index 0fc8c29a5b7..0eee36eec21 100644 --- a/src/hotspot/share/cds/archiveBuilder.cpp +++ b/src/hotspot/share/cds/archiveBuilder.cpp @@ -39,6 +39,7 @@ #include "classfile/systemDictionaryShared.hpp" #include "classfile/vmClasses.hpp" #include "interpreter/abstractInterpreter.hpp" +#include "jvm.h" #include "logging/log.hpp" #include "logging/logStream.hpp" #include "memory/allStatic.hpp" @@ -170,7 +171,7 @@ ArchiveBuilder::ArchiveBuilder() : { _klasses = new (mtClassShared) GrowableArray(4 * K, mtClassShared); _symbols = new (mtClassShared) GrowableArray(256 * K, mtClassShared); - + _entropy_seed = 0x12345678; assert(_current == nullptr, "must be"); _current = this; } @@ -190,6 +191,16 @@ ArchiveBuilder::~ArchiveBuilder() { } } +// Returns a deterministic sequence of pseudo random numbers. The main purpose is NOT +// for randomness but to get good entropy for the identity_hash() of archived Symbols, +// while keeping the contents of static CDS archives deterministic to ensure +// reproducibility of JDK builds. +int ArchiveBuilder::entropy() { + assert(SafepointSynchronize::is_at_safepoint(), "needed to ensure deterministic sequence"); + _entropy_seed = os::next_random(_entropy_seed); + return static_cast(_entropy_seed); +} + class GatherKlassesAndSymbols : public UniqueMetaspaceClosure { ArchiveBuilder* _builder; diff --git a/src/hotspot/share/cds/archiveBuilder.hpp b/src/hotspot/share/cds/archiveBuilder.hpp index a80370d3976..e6ac2e7ac4e 100644 --- a/src/hotspot/share/cds/archiveBuilder.hpp +++ b/src/hotspot/share/cds/archiveBuilder.hpp @@ -219,6 +219,7 @@ private: ResizeableResourceHashtable _buffered_to_src_table; GrowableArray* _klasses; GrowableArray* _symbols; + unsigned int _entropy_seed; // statistics DumpAllocStats _alloc_stats; @@ -341,6 +342,7 @@ public: ArchiveBuilder(); ~ArchiveBuilder(); + int entropy(); void gather_klasses_and_symbols(); void gather_source_objs(); bool gather_klass_and_symbol(MetaspaceClosure::Ref* ref, bool read_only); diff --git a/src/hotspot/share/cds/metaspaceShared.cpp b/src/hotspot/share/cds/metaspaceShared.cpp index 231f69e3ab1..b13ef2101a9 100644 --- a/src/hotspot/share/cds/metaspaceShared.cpp +++ b/src/hotspot/share/cds/metaspaceShared.cpp @@ -513,9 +513,6 @@ void VM_PopulateDumpSharedSpace::doit() { char* cloned_vtables = CppVtables::dumptime_init(&builder); - // Initialize random for updating the hash of symbols - os::init_random(0x12345678); - builder.dump_rw_metadata(); builder.dump_ro_metadata(); builder.relocate_metaspaceobj_embedded_pointers(); diff --git a/src/hotspot/share/oops/symbol.cpp b/src/hotspot/share/oops/symbol.cpp index cbb51e1bb90..4deae372061 100644 --- a/src/hotspot/share/oops/symbol.cpp +++ b/src/hotspot/share/oops/symbol.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,6 +23,7 @@ */ #include "precompiled.hpp" +#include "cds/archiveBuilder.hpp" #include "cds/metaspaceShared.hpp" #include "classfile/altHashing.hpp" #include "classfile/classLoaderData.hpp" @@ -73,11 +74,8 @@ Symbol::Symbol(const Symbol& s1) { #if INCLUDE_CDS void Symbol::update_identity_hash() { - // This is called at a safepoint during dumping of a static CDS archive. The caller should have - // called os::init_random() with a deterministic seed and then iterate all archived Symbols in - // a deterministic order. assert(SafepointSynchronize::is_at_safepoint(), "must be at a safepoint"); - _hash_and_refcount = pack_hash_and_refcount((short)os::random(), PERM_REFCOUNT); + _hash_and_refcount = pack_hash_and_refcount((short)ArchiveBuilder::current()->entropy(), PERM_REFCOUNT); } void Symbol::set_permanent() {