On Thu, 16 Jan 2025 22:41:06 GMT, David Holmes <dhol...@openjdk.org> wrote:
> I'm really not sure what this is actually trying to benchmark - who will be > collecting this data and analysing it and what will they do with their > results? > > That aside, generating the logs in a training run is okay, but the tool for > processing those logs should reside elsewhere. > > A few initial passing comments. Thank you David for taking a look - very appreciated! The processing tools are indeed somewhere else - `jdk/src/utils` This benchmark records chronologically all the `malloc/realloc/free` calls so teey ca be played back later, which allows developers to test their code before/after with respect to malloc calls to measure speed/memory usage. I believe it should be a requirement to run this benchmark with your changes both to look for speed, but also memory usage, ebfore checkign in anything (though we can start with NTM related code) It was already used successfully to optimize Treap. The main idea behind it is that you need to record the pattern only once (using some fancy expensive use case), then iterate quickly over your fix and test against that pattern, without needing to actually run that use case. > src/demo/share/jfc/J2Ddemo/java2d/Intro.java line 449: > >> 447: System.exit(0); >> 448: } >> 449: Scene scene = director.get(index); > > Leftover debugging code? Yes, it is. I wanted a way to quit Java2Demo at the exact same point. I will revert it. > src/hotspot/share/nmt/memLogRecorder.cpp line 43: > >> 41: // then to actually run the benchmark: >> 42: // >> 43: // ./build/macosx-aarch64-server-release/xcode/build/jdk/bin/java >> -XX:+UnlockDiagnosticVMOptions -XX:NMTBenchmarkRecordedPID=43100 >> -XX:NMTBenchmarkRecordedLoops=10 > > ?? How does this run anything - you haven't passed an initial class to the > Java invocation? We check for the flags when we initialize NMT and run only when NMTBenchmarkRecordedPID is set. > src/hotspot/share/nmt/memLogRecorder.hpp line 37: > >> 35: #elif defined(WINDOWS) >> 36: // ??? >> 37: #endif > > You need to use existing synchronization API here. Either Mutex or > PlatformMutex depending on initialization constraints; maybe a Semaphore if > the others can't work. Why do we need to use existing synchronization API here? IS it because we can deadlock by introducing a new lock? I have run the existing code many times, no deadlocks. ------------- PR Comment: https://git.openjdk.org/jdk/pull/23115#issuecomment-2598729741 PR Review Comment: https://git.openjdk.org/jdk/pull/23115#discussion_r1920446882 PR Review Comment: https://git.openjdk.org/jdk/pull/23115#discussion_r1920445805 PR Review Comment: https://git.openjdk.org/jdk/pull/23115#discussion_r1920448480
