On Mon, 2 Aug 2021 14:24:54 GMT, Coleen Phillimore wrote:
>>> This is an interesting and it seems a better way to solve this problem.
>>> Where were you all those years ago?? I hope @zhengyu123 has a chance to
>>> review it.
>>
>> Thank you! I was here, but we were not yet doing much upstream
On Wed, 4 Aug 2021 03:36:11 GMT, Thomas Stuefe wrote:
> Nightlies at SAP showed no problems for several runs now. The failed GHA test
> (StringDeduplication) seems to have nothing to do with my patch.
>
> @zhengyu123 are you fine with the latest version of this patch?
Still good.
On Sun, 1 Aug 2021 08:17:08 GMT, Thomas Stuefe wrote:
>> Short: this patch makes NMT available in custom-launcher scenarios and
>> during gtests. It simplifies NMT initialization. It adds a lot of
>> NMT-specific testing, cleans them up and makes them sideeffect-free.
>>
>> -
>>
>>
On Mon, 2 Aug 2021 14:38:23 GMT, Coleen Phillimore wrote:
> This looks good. Thanks for fixing the mysterious (to me) cast.
Thank you, Coleen!
-
PR: https://git.openjdk.java.net/jdk/pull/4874
On Sun, 1 Aug 2021 08:17:08 GMT, Thomas Stuefe wrote:
>> Short: this patch makes NMT available in custom-launcher scenarios and
>> during gtests. It simplifies NMT initialization. It adds a lot of
>> NMT-specific testing, cleans them up and makes them sideeffect-free.
>>
>> -
>>
>>
On Fri, 30 Jul 2021 09:32:22 GMT, Thomas Stuefe wrote:
>> [Not a review, just a drive-by comment.] This is a normal and idiomatic
>> overload on the const-ness of `this`.
>
> To expand on Kim's answer. This is a way to solve const/nonconst problems
> like
On Fri, 30 Jul 2021 09:44:57 GMT, Thomas Stuefe wrote:
> Is that a real-life problem? Are there cases where the launcher would want to
> live on if the JVM failed to load?
There are a lot of other reasons why the launcher couldn't live on if the JVM
fails to load. This was only one of them.
On Sun, 1 Aug 2021 08:17:08 GMT, Thomas Stuefe wrote:
>> Short: this patch makes NMT available in custom-launcher scenarios and
>> during gtests. It simplifies NMT initialization. It adds a lot of
>> NMT-specific testing, cleans them up and makes them sideeffect-free.
>>
>> -
>>
>>
On Fri, 30 Jul 2021 20:14:04 GMT, Zhengyu Gu wrote:
>> Thomas Stuefe has updated the pull request incrementally with six additional
>> commits since the last revision:
>>
>> - feedback zhengyu
>> - feeback Coleen/Kim (constness fix)
>> - Feedback David
>> - Add test to test for non-java
> Short: this patch makes NMT available in custom-launcher scenarios and during
> gtests. It simplifies NMT initialization. It adds a lot of NMT-specific
> testing, cleans them up and makes them sideeffect-free.
>
> -
>
> NMT continues to be an extremely useful tool for SAP to tackle
On Thu, 29 Jul 2021 06:33:38 GMT, David Holmes wrote:
>> Short: this patch makes NMT available in custom-launcher scenarios and
>> during gtests. It simplifies NMT initialization. It adds a lot of
>> NMT-specific testing, cleans them up and makes them sideeffect-free.
>>
>> -
>>
>>
On Thu, 22 Jul 2021 14:58:47 GMT, Thomas Stuefe wrote:
> Short: this patch makes NMT available in custom-launcher scenarios and during
> gtests. It simplifies NMT initialization. It adds a lot of NMT-specific
> testing, cleans them up and makes them sideeffect-free.
>
> -
>
> NMT
On Fri, 30 Jul 2021 18:28:44 GMT, Zhengyu Gu wrote:
>> So, you are saying that there is no memory that is malloc'd pre-NMT-init
>> phase and freed post-NMT-init phase?
>
> Okay, I see. It just leaks those memory, so the table can be read-only.
Exactly. There are a few allocs that either get
On Fri, 30 Jul 2021 16:36:41 GMT, Thomas Stuefe wrote:
>> src/hotspot/share/services/nmtPreInit.hpp line 309:
>>
>>> 307: ::memcpy(p_new, a->payload(), MIN2(a->size, new_size));
>>> 308: (*rc) = p_new;
>>> 309: _num_reallocs_pre_to_post++;
>>
>> post-NMT-init counter
On Fri, 30 Jul 2021 16:33:17 GMT, Zhengyu Gu wrote:
>> The code is implicitly thread-safe because the hashmap is only modified in
>> the pre-NMT-init phase. After NMT initialization, the table is read-only.
>> During pre-NMT-init we are effectively single-threaded - at most two threads
>>
On Fri, 30 Jul 2021 16:28:54 GMT, Thomas Stuefe wrote:
>> src/hotspot/share/services/nmtPreInit.hpp line 202:
>>
>>> 200: assert((*aa) != NULL, "Entry not found: " PTR_FORMAT, p2i(p));
>>> 201: NMTPreInitAllocation* a = (*aa);
>>> 202: (*aa) = (*aa)->next; // remove from its
On Fri, 30 Jul 2021 13:03:32 GMT, Zhengyu Gu wrote:
> Sorry for late review.
>
> Did a quick scan and have a few questions, will do more detail reading later.
Thanks a lot, I appreciate your feedback!
> src/hotspot/share/services/nmtPreInit.hpp line 108:
>
>> 106: // - lookup speed is
On Fri, 30 Jul 2021 12:56:59 GMT, Zhengyu Gu wrote:
>> Short: this patch makes NMT available in custom-launcher scenarios and
>> during gtests. It simplifies NMT initialization. It adds a lot of
>> NMT-specific testing, cleans them up and makes them sideeffect-free.
>>
>> -
>>
>> NMT
On Thu, 22 Jul 2021 14:58:47 GMT, Thomas Stuefe wrote:
> Short: this patch makes NMT available in custom-launcher scenarios and during
> gtests. It simplifies NMT initialization. It adds a lot of NMT-specific
> testing, cleans them up and makes them sideeffect-free.
>
> -
>
> NMT
On Fri, 30 Jul 2021 04:09:32 GMT, Kim Barrett wrote:
>> src/hotspot/share/services/nmtPreInit.hpp line 166:
>>
>>> 164: NMTPreInitAllocation** find_entry(const void* p) const {
>>> 165: const unsigned index = index_for_key(p);
>>> 166: NMTPreInitAllocation** aa =
On Thu, 29 Jul 2021 23:03:46 GMT, Coleen Phillimore wrote:
> This is an interesting and it seems a better way to solve this problem. Where
> were you all those years ago?? I hope @zhengyu123 has a chance to review it.
Thank you! I was here, but we were not yet doing much upstream :) To be
On Thu, 29 Jul 2021 06:37:36 GMT, David Holmes wrote:
> Hi Thomas,
>
> I had a look through this and it seems reasonable, but I'm not familiar
> enough with the details to approve at this stage.
>
> A few nits below.
>
> Thanks,
> David
I did not expect a quick review for this one, so
On Fri, 30 Jul 2021 04:03:57 GMT, Kim Barrett wrote:
>> src/hotspot/share/services/nmtPreInit.hpp line 128:
>>
>>> 126: // Returns start of the user data area
>>> 127: void* payload() { return this + 1; }
>>> 128: const void* payload() const { return this + 1; }
>>
>> This is
On Thu, 29 Jul 2021 22:53:42 GMT, Coleen Phillimore wrote:
>> Short: this patch makes NMT available in custom-launcher scenarios and
>> during gtests. It simplifies NMT initialization. It adds a lot of
>> NMT-specific testing, cleans them up and makes them sideeffect-free.
>>
>> -
>>
On Thu, 29 Jul 2021 22:52:07 GMT, Coleen Phillimore wrote:
>> Short: this patch makes NMT available in custom-launcher scenarios and
>> during gtests. It simplifies NMT initialization. It adds a lot of
>> NMT-specific testing, cleans them up and makes them sideeffect-free.
>>
>> -
>>
On Thu, 22 Jul 2021 14:58:47 GMT, Thomas Stuefe wrote:
> Short: this patch makes NMT available in custom-launcher scenarios and during
> gtests. It simplifies NMT initialization. It adds a lot of NMT-specific
> testing, cleans them up and makes them sideeffect-free.
>
> -
>
> NMT
On Thu, 22 Jul 2021 14:58:47 GMT, Thomas Stuefe wrote:
> Short: this patch makes NMT available in custom-launcher scenarios and during
> gtests. It simplifies NMT initialization. It adds a lot of NMT-specific
> testing, cleans them up and makes them sideeffect-free.
>
> -
>
> NMT
On Thu, 22 Jul 2021 14:58:47 GMT, Thomas Stuefe wrote:
> Short: this patch makes NMT available in custom-launcher scenarios and during
> gtests. It simplifies NMT initialization. It adds a lot of NMT-specific
> testing, cleans them up and makes them sideeffect-free.
>
> -
>
> NMT
On 28/07/2021 12:17 am, Thomas Stuefe wrote:
On Mon, 26 Jul 2021 21:08:04 GMT, David Holmes wrote:
Before looking at this, have you checked the startup performance impact?
Thanks,
David
-
Hi David,
performance should not be a problem. The potentially costly thing is the
underlying
On Mon, 26 Jul 2021 21:08:04 GMT, David Holmes wrote:
> Before looking at this, have you checked the startup performance impact?
>
> Thanks,
> David
> -
Hi David,
performance should not be a problem. The potentially costly thing is the
underlying hashmap. But we keep it operating with a
Hi Thomas,
On 26/07/2021 10:15 pm, Thomas Stuefe wrote:
Short: this patch makes NMT available in custom-launcher scenarios and during
gtests. It simplifies NMT initialization. It adds a lot of NMT-specific testing.
Before looking at this, have you checked the startup performance impact?
Short: this patch makes NMT available in custom-launcher scenarios and during
gtests. It simplifies NMT initialization. It adds a lot of NMT-specific testing.
-
NMT continues to be an extremely useful tool for SAP to tackle memory problems
in the JVM.
However, NMT is of limited use
32 matches
Mail list logo
