On Fri, 9 Aug 2024 10:14:10 GMT, Johan Sjölen 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 in the code an
On Tue, 20 Aug 2024 07:33:06 GMT, Afshin Zafari wrote:
>> src/hotspot/share/nmt/nmtNativeCallStackStorage.hpp line 94:
>>
>>> 92: if (si._stack_index < 0 || si._stack_index >= _stacks.length()) {
>>> 93: return _fake_stack;
>>> 94: }
>>
>> Is that a leftover from debugging?
>>
>>
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
On Mon, 5 Aug 2024 17:23:46 GMT, Afshin Zafari wrote:
>> It is considered that `malloc` or other external events are the same for two
>> cases. If we know that there might be some noise for one or another, we
>> should check and disable them. This is the approach I have talked. How can
>> we a
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_000, 2)` units of
>> w
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
>>
On Mon, 5 Aug 2024 16:54:40 GMT, Afshin Zafari wrote:
>> This applies the reserve/commit regardless of outcome, so slightly different.
>
> 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
>
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
On Mon, 5 Aug 2024 08:41:49 GMT, Johan Sjölen wrote:
>> In `MemoryFileTracker`, the changes in commit/reserve are applied to a local
>> `VirtualMemorySnapshot`. Here we have to apply them to the global
>> `VirtualMemorySummary`.
>
> Yeah, that doesn't seem like a problem.
>
> ```c++
> for (i
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
On Mon, 5 Aug 2024 16:50:00 GMT, Afshin Zafari wrote:
>> The bool argument is just passed along.
>>
>> ```c++
>> RegionsTree(bool with_storage) : VMATree(), _ncs_storage(with_storage) {
>> }
>
> Done.
> For my curiosity, what is the advantage?
1. No malloc
2. No indirection, so no cache mis
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.
--
On Mon, 14 Oct 2024 08:50:52 GMT, Afshin Zafari wrote:
>> Why is this assert triggered? That sounds like a bug.
>
> The assertion that happens during building jdk-image:
>
> #
> # A fatal error has been detected by the Java Runtime Environment:
> #
> # Internal Error
> (/home/afshin/scratch/83
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
On Fri, 27 Sep 2024 09:57:35 GMT, Afshin Zafari wrote:
>> Removed.
>
> 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 < _
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()) {
>>
>> I don't think this should be here?
>
On Wed, 14 Aug 2024 19:11:58 GMT, Gerard Ziemski 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 in the code
On Wed, 14 Aug 2024 19:13:49 GMT, Gerard Ziemski wrote:
>> src/hotspot/share/nmt/memReporter.cpp line 467:
>>
>>> 465:
>>> 466: if (reserved_and_committed)
>>> 467: return;
>>
>> This looks better, but now that I got more comfortable with anonymous local
>> functions using lambd
On Thu, 7 Nov 2024 12:48:12 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 in the code a
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
On Mon, 14 Oct 2024 13:22:54 GMT, Afshin Zafari 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 r
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
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?
On Mon, 14 Oct 2024 10:24:54 GMT, Afshin Zafari wrote:
> > ```c++
> > region->add_committed_region(committed_start, committed_size, ncs); // <--
> > LOST!
> > ```
>
> The `region` is not a VMATree::node, it is a `ReservedMemoryRegion*`.
I don't understand what you're trying to say here. Do you
On Fri, 27 Sep 2024 16:10:59 GMT, Gerard Ziemski wrote:
> > > Rather than having 2 implementations, I'd like to see us aiming for
> > > integration for JDK-25 after forking 24, so integration in December. That
> > > would give us 6 months of ensuring stability of the new implementation
> > > b
On Fri, 27 Sep 2024 16:05:33 GMT, Afshin Zafari wrote:
> > Rather than having 2 implementations, I'd like to see us aiming for
> > integration for JDK-25 after forking 24, so integration in December. That
> > would give us 6 months of ensuring stability of the new implementation
> > before it
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
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
On Fri, 27 Sep 2024 10:22:57 GMT, Afshin Zafari 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
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
On Fri, 27 Sep 2024 09:52:07 GMT, Afshin Zafari 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
> > tes
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
>
On Mon, 26 Aug 2024 11:03:25 GMT, Johan Sjölen wrote:
> It's tedious to have to write return true; if you never return false; in the
> function. I looked into fixing this with some metaprogramming, and I found
> two C++14 solutions and one C++17 one. Here's the Godbolt
I'd like the lambdas wit
On Mon, 26 Aug 2024 07:16:39 GMT, Afshin Zafari 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
On Fri, 9 Aug 2024 16:05:00 GMT, Gerard Ziemski wrote:
> > > Is there a concrete advantage here for using lambda expression when
> > > iterating committed regions? I'm asking because personally I find
> > > ```c++
> > > while ((committed_rgn = itr.next()) != nullptr) {
> > > print_comm
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
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 in the code and ca
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 in the code and ca
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 in the code and ca
- `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
40 matches
Mail list logo
