Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Tue, 6 Aug 2024 07:12:13 GMT, Johan Sjölen wrote: >> When it is said that an algorithm has the log(n) time-complexity, it means >> that if the input grows n times, the times grows log(n) times. The tree >> data-structure has log(n) time-complexity. VMATree may have not exactly >> log(n) res

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Mon, 5 Aug 2024 17:20:24 GMT, Afshin Zafari wrote: >> Why would the execution time grow logarithmically when we do linearly more >> work? When we run this with `N2` we will perform `10_000 * log(10_000, 2)` >> units of work, and for `N1` we will perform `1_000 * log(1

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Fri, 9 Aug 2024 10:03:52 GMT, Johan Sjölen wrote: >> The main purpose of the `if (...)` cases is to find if the request to apply >> the delta is valid or not. There are related assertions in VirtualMemory but >> not so informative. Also, using `log_debug` lets the build proceed and just >>

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Mon, 5 Aug 2024 08:42:43 GMT, Johan Sjölen wrote: >> Yeah, that doesn't seem like a problem. >> >> ```c++ >> for (int i = 0; i < mt_number_of_types; i++) { >> r = diff.flag[i].reserve; >> c = diff.flag[i].commit; >> flag = NMTUtil::index_to_flag(i); >> VirtualMemory* mem = V

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Fri, 9 Aug 2024 15:05:59 GMT, Johan Sjölen wrote: >> src/hotspot/share/nmt/regionsTree.hpp line 46: >> >>> 44: using Node = VMATree::TreapNode; >>> 45: >>> 46: class NodeHelper : public Node { >> >> This shouldn't inherit from `Node` and then have each instance be cast into >> `NodeHel

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Fri, 9 Aug 2024 09:05:37 GMT, Johan Sjölen wrote: >> Done. >> For my curiosity, what is the advantage? > > 1. No malloc > 2. No indirection, so no cache misses > 3. A clear lifetime and clear ownership, both are bound to the `RegionsTree` > object OK. Thanks for your description. --

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Sat, 12 Oct 2024 14:24:15 GMT, Johan Sjölen wrote: >> No, returned back. >> This assert is triggered: >> >> # Internal Error >> (/home/afshin/scratch/833XX_nmt_VMT_with_tree/src/hotspot/share/utilities/growableArray.hpp:142), >> pid=1972883, tid=1972931 >> # assert(0 <= i && i < _len) fai

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Fri, 27 Sep 2024 09:46:27 GMT, Afshin Zafari wrote: >> src/hotspot/share/nmt/nmtNativeCallStackStorage.hpp line 93: >> >>> 91: >>> 92: const inline NativeCallStack& get(StackIndex si) { >>> 93: if (is_invalid(si) || si >= _stacks.length

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Mon, 14 Oct 2024 18:56:47 GMT, Johan Sjölen wrote: > I think it's actually the opposite: None of the committed regions will > survive after this function. You maybe missed my point when said " ... some extra committed size in NMT reports". I emphasize on the " the NMT reports", since the co

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Mon, 14 Oct 2024 12:37:05 GMT, Johan Sjölen wrote: > Yes, but this code is incorrect. So we should have a test detecting this, but > we do not, and so this is under-tested. This code finds committed regions within a Stack region if they are not accounted by NMT so far, IIUC. By running this

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Fri, 27 Sep 2024 16:23:29 GMT, Gerard Ziemski wrote: > Should we say then, that this is blocked by those 2 issues? Is it OK then if > I wait till those get checked in before verifying the performance benefits if > the new implementation? The performance was the main motivation here, correct?

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Fri, 27 Sep 2024 10:10:37 GMT, Johan Sjölen wrote: > Sure, I can understand that it's nice to have both versions present during > development. Right now it seems like we have a broken build, do you have any > plans on having a functioning and fully featured build soon? The new commit is pus

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Fri, 27 Sep 2024 11:43:19 GMT, Johan Sjölen wrote: > Why should the default be disabled? That requires customers to explicitly > pick the new tree to be used, which they are very unlikely to do. As I > understand it, and correct me if I am wrong, the main goal of having two > implementation

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Fri, 27 Sep 2024 09:49:37 GMT, Johan Sjölen wrote: > The fork for JDK-24 is on December 5th. That means that we have at most 8 > weeks in the testing system to find and fix any bugs that we might have > missed after integration. To me, that feels a bit short. Either we wait after > the fork

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Thu, 26 Sep 2024 10:58:31 GMT, Johan Sjölen wrote: > What is the consensus on having two implementations alive at the same time? > I'd like to see us delete the old VirtualMemoryTracker and only have the tree > implementation left as part of this PR. What is the status of our testing? Is >

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Fri, 23 Aug 2024 21:02:25 GMT, Gerard Ziemski wrote: > Is the plan to check-in the fix with both paths? Or are we going to remove > the linked-list based one after the review? The plan is to have both versions available at run-time. In this plan, we will add JVM options to let the user to s

Re: RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
On Thu, 1 Aug 2024 15:44:32 GMT, Afshin Zafari wrote: > - `VMATree` is used instead of `SortedLinkList` in new class > `VirtualMemoryTrackerWithTree`. > - A wrapper/helper `RegionTree` is made around VMATree to make some calls > easier. > - Both old and new versions exist

RFR: 8337217: Port VirtualMemoryTracker to use VMATree

2024-11-08 Thread Afshin Zafari
- `VMATree` is used instead of `SortedLinkList` in new class `VirtualMemoryTrackerWithTree`. - A wrapper/helper `RegionTree` is made around VMATree to make some calls easier. - Both old and new versions exist in the code and can be selected via `MemTracker::set_version()` - `find_reserved_r

Re: RFR: 8314438: NMT: Performance benchmarks are needed to have a baseline for comparison of improvements [v2]

2023-09-25 Thread Afshin Zafari
On Mon, 25 Sep 2023 07:41:24 GMT, Alan Bateman wrote: >> Afshin Zafari has updated the pull request incrementally with one additional >> commit since the last revision: >> >> new benchmark moved to vm/runtime folder. > > test/micro/org/openjdk/bench/java/

Re: RFR: 8314438: NMT: Performance benchmarks are needed to have a baseline for comparison of improvements [v2]

2023-09-25 Thread Afshin Zafari
penjdk.org/browse/JDK-8316813) is > created for measuring the virtual memory tracing parts of NMT. Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision: new benchmark moved to vm/runtime folder. - Changes: -

RFR: 8314438: NMT: Performance benchmarks are needed to have a baseline for comparison of improvements

2023-09-25 Thread Afshin Zafari
A new benchmark for measuring the NMT overhead in `summary` and `detail` modes. The tests are run using: make CONF=debug test TEST="micro:java.util.NMTBenchmark" MICRO="RESULTS_FORMAT=json" The results are written to a JSON file that can be visualized using [JMH Visualizer](https://jmh.moreth

Re: RFR: 8314438: NMT: Performance benchmarks are needed to have a baseline for comparison of improvements

2023-09-25 Thread Afshin Zafari
On Wed, 6 Sep 2023 19:56:10 GMT, Gerard Ziemski wrote: > The test will not compile for me unless I add: > > `import java.util.concurrent.TimeUnit;` Sorry, it was removed mistakenly before pushing the code. Now fixed. > You said `The JSON file can be used for visualising the results.` Can you

Integrated: 8301622: ProcessTools.java compilation gets ThreadDeath deprecation warning

2023-03-03 Thread Afshin Zafari
On Thu, 2 Mar 2023 12:15:21 GMT, Afshin Zafari wrote: > ### Description > The use of `ThreadDeath` is replaced with checking the exception be of type > `Error` and the thread is `TERMINATED`. > > > ### Test > local and mach5 tier1 This pull request has now been i

Re: RFR: 8301622: ProcessTools.java compilation gets ThreadDeath deprecation warning [v2]

2023-03-02 Thread Afshin Zafari
On Thu, 2 Mar 2023 17:35:35 GMT, Alan Bateman wrote: >> I assumed the `ThreadDeath` was there to catch `Thread.stop()` calls and >> just ignore them. So, I wanted to keep this functionality. >> Can we remove using `ThreadDeath` relying on that `Thread.stop()` is also >> deprecated? > > I think

Re: RFR: 8301622: ProcessTools.java compilation gets ThreadDeath deprecation warning [v2]

2023-03-02 Thread Afshin Zafari
> ### Description > The use of `ThreadDeath` is replaced with checking the exception be of type > `Error` and the thread is `TERMINATED`. > > > ### Test > local and mach5 tier1 Afshin Zafari has updated the pull request incrementally with one additional commit si

Re: RFR: 8301622: ProcessTools.java compilation gets ThreadDeath deprecation warning

2023-03-02 Thread Afshin Zafari
On Thu, 2 Mar 2023 14:13:10 GMT, Alan Bateman wrote: >> ### Description >> The use of `ThreadDeath` is replaced with checking the exception be of type >> `Error` and the thread is `TERMINATED`. >> >> >> ### Test >> local and mach5 tier1 > > test/lib/jdk/test/lib/process/ProcessTools.java line

RFR: 8301622: ProcessTools.java compilation gets ThreadDeath deprecation warning

2023-03-02 Thread Afshin Zafari
### Description The use of `ThreadDeath` is replaced with checking the exception be of type `Error` and the thread is `TERMINATED`. ### Test local and mach5 tier1 - Commit messages: - 8301622: ProcessTools.java compilation gets ThreadDeath deprecation warning Changes: https://git