> Please review this small cleanup code, removing the now unused allocation
> type from the fieldInfo structure.
>
> Tested with Mach5, tiers 1 to 3 and locally by running
> test/hotspot/jtreg/serviceability/sa tests.
>
> Thank you,
>
> Fred
Frederic Parain has updated the pull request
On Mon, 9 Nov 2020 19:53:32 GMT, Harold Seigel wrote:
>> Frederic Parain has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove unused symbol from vmStruct
>
> src/hotspot/share/oops/fieldInfo.hpp line 61:
>
>> 59: //
On Mon, 9 Nov 2020 08:45:49 GMT, Erik Österlund wrote:
>> @fisk - I'm leaving this one for you for now.
>
> Changing it to a do/while loop makes sense. The while condition is always
> true the first iteration, so doing a while or do/while loop is equivalent. If
> you find the do/while loop
> Please review this small cleanup code, removing the now unused allocation
> type from the fieldInfo structure.
>
> Tested with Mach5, tiers 1 to 3 and locally by running
> test/hotspot/jtreg/serviceability/sa tests.
>
> Thank you,
>
> Fred
Frederic Parain has updated the pull request
On Fri, 6 Nov 2020 02:40:44 GMT, David Holmes wrote:
>> Daniel D. Daugherty has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Resolve more @dholmes-ora comments with help from @fisk.
>
> Hi Dan,
>
> Overall this looks great. Comparing
On Mon, 9 Nov 2020 03:19:17 GMT, Daniel D. Daugherty wrote:
>> @dholmes-ora - Thanks for the review! Hmm... I'm not sure why the GitHub UI
>> send out my replies one-at-a-time. Perhaps I should have replied from the
>> "files" view instead of the main PR view?
>
> It is a deflated monitor that
On Wed, 21 Oct 2020 17:37:20 GMT, Aleksey Shipilev wrote:
>> Good.
>
> Thanks for review, @kvn! I would also like a review from someone from
> serviceability.
Friendly reminder.
-
PR: https://git.openjdk.java.net/jdk/pull/650
On Thu, 5 Nov 2020 12:42:40 GMT, Coleen Phillimore wrote:
>> Thank you for the update, Coleen!
>> I leave it for you to decide to refactor the gc_notification or not.
>> Thanks,
>> Serguei
>
> Thanks @sspitsyn . I'm going to leave the gc_notification code because
> structurally the two sides
On Mon, 9 Nov 2020 20:05:09 GMT, Frederic Parain wrote:
>> Please review this small cleanup code, removing the now unused allocation
>> type from the fieldInfo structure.
>>
>> Tested with Mach5, tiers 1 to 3 and locally by running
>> test/hotspot/jtreg/serviceability/sa tests.
>>
>> Thank
On 10/11/2020 6:39 am, Daniel D.Daugherty wrote:
On Mon, 9 Nov 2020 08:34:56 GMT, Erik Österlund wrote:
I noticed that in my preliminary review of Erik's changes. He checked
with the JFR guys and they said it just needed to be an address and
does not have to refer to the Object.
@fisk - can
On Mon, 9 Nov 2020 08:34:56 GMT, Erik Österlund wrote:
>> I noticed that in my preliminary review of Erik's changes. He checked
>> with the JFR guys and they said it just needed to be an address and
>> does not have to refer to the Object.
>>
>> @fisk - can you think of a comment we should add
On Mon, 9 Nov 2020 20:51:17 GMT, Daniel D. Daugherty wrote:
>> Changes from @fisk and @dcubed-ojdk to:
>>
>> - simplify ObjectMonitor list management
>> - get rid of Type-Stable Memory (TSM)
>>
>> This change has been tested with Mach5 Tier[1-3],4,5,6,7,8; no new
>> regressions.
>> Aurora
On Mon, 9 Nov 2020 19:54:09 GMT, Lois Foltan wrote:
>> Frederic Parain has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove unused symbol from vmStruct
>
> Marked as reviewed by lfoltan (Reviewer).
Looks good.
-
PR:
On Mon, 9 Nov 2020 19:54:58 GMT, Frederic Parain wrote:
>> Please review this small cleanup code, removing the now unused allocation
>> type from the fieldInfo structure.
>>
>> Tested with Mach5, tiers 1 to 3 and locally by running
>> test/hotspot/jtreg/serviceability/sa tests.
>>
>> Thank
On Mon, 9 Nov 2020 16:36:07 GMT, Claes Redestad wrote:
>> Frederic Parain has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove unused symbol from vmStruct
>
> Nice cleanup!
Hi Claes,
Thank you for your review, the new version should
On Mon, 9 Nov 2020 19:54:58 GMT, Frederic Parain wrote:
>> Please review this small cleanup code, removing the now unused allocation
>> type from the fieldInfo structure.
>>
>> Tested with Mach5, tiers 1 to 3 and locally by running
>> test/hotspot/jtreg/serviceability/sa tests.
>>
>> Thank
Hi David,
I'm going to try replying to this review comment via email to see how that
works out...
On 11/9/20 1:06 AM, David Holmes wrote:
Hi Dan,
On 9/11/2020 1:50 pm, Daniel D.Daugherty wrote:
On Sun, 8 Nov 2020 21:43:00 GMT, David Holmes
wrote:
How about this:
static MonitorList
> Changes from @fisk and @dcubed-ojdk to:
>
> - simplify ObjectMonitor list management
> - get rid of Type-Stable Memory (TSM)
>
> This change has been tested with Mach5 Tier[1-3],4,5,6,7,8; no new
> regressions.
> Aurora Perf runs have also been done (DaCapo-h2, Quick Startup/Footprint,
>
Hi Dan,
I'm going to top-post just to keep this short :)
Thanks for all the details below on the performance aspects and use of
the heuristics - all good.
Regarding "ceiling" versus "watermark" ... a "high watermark" is a
ceiling so its really a matter of personal preference what
On Mon, 9 Nov 2020 09:03:37 GMT, Erik Österlund wrote:
>> @fisk - this one is for you... :-)
>
> Yeah this is one of the new cool features we can use. I thought it is
> allowed, because it is neither in the excluded nor undecided list of features
> in our doc/hotspot-style.md file.
On 9/11/2020 7:05 pm, Erik Österlund wrote:
On Sat, 7 Nov 2020 17:22:21 GMT, Daniel D. Daugherty wrote:
src/hotspot/share/runtime/synchronizer.cpp line 1520:
1518: // deflated in this cycle.
1519: size_t deleted_count = 0;
1520: for (ObjectMonitor* monitor: delete_list) {
I
On 11/9/20 6:25 PM, David Holmes wrote:
On Mon, 9 Nov 2020 20:51:17 GMT, Daniel D. Daugherty wrote:
Changes from @fisk and @dcubed-ojdk to:
- simplify ObjectMonitor list management
- get rid of Type-Stable Memory (TSM)
This change has been tested with Mach5 Tier[1-3],4,5,6,7,8; no new
> Please review this small cleanup code, removing the now unused allocation
> type from the fieldInfo structure.
>
> Tested with Mach5, tiers 1 to 3 and locally by running
> test/hotspot/jtreg/serviceability/sa tests.
>
> Thank you,
>
> Fred
Frederic Parain has updated the pull request
On Mon, 9 Nov 2020 20:42:19 GMT, Daniel D. Daugherty wrote:
>> Changing it to a do/while loop makes sense. The while condition is always
>> true the first iteration, so doing a while or do/while loop is equivalent.
>> If you find the do/while loop easier to read, then that sounds good to me.
>
On Tue, 10 Nov 2020 01:03:00 GMT, David Holmes wrote:
>> Okay. I've changed it to a do-while loop.
>
> The loop that was being discussed here is the one on line 93 of the original
> changeset:
> `while (next != NULL && next->is_being_async_deflated()) {`
> I made no comment on the outer while
On 10/11/2020 12:35 pm, Daniel D.Daugherty wrote:
On Tue, 10 Nov 2020 02:15:19 GMT, Daniel D. Daugherty
wrote:
The loop that was being discussed here is the one on line 93 of the original
changeset:
`while (next != NULL && next->is_being_async_deflated()) {`
I made no comment on the outer
On Tue, 10 Nov 2020 02:35:17 GMT, Daniel D. Daugherty
wrote:
>> Changes from @fisk and @dcubed-ojdk to:
>>
>> - simplify ObjectMonitor list management
>> - get rid of Type-Stable Memory (TSM)
>>
>> This change has been tested with Mach5 Tier[1-3],4,5,6,7,8; no new
>> regressions.
>> Aurora
On Tue, 10 Nov 2020 02:15:19 GMT, Daniel D. Daugherty
wrote:
>> The loop that was being discussed here is the one on line 93 of the original
>> changeset:
>> `while (next != NULL && next->is_being_async_deflated()) {`
>> I made no comment on the outer while loop.
>
> You're correct. I got
> Changes from @fisk and @dcubed-ojdk to:
>
> - simplify ObjectMonitor list management
> - get rid of Type-Stable Memory (TSM)
>
> This change has been tested with Mach5 Tier[1-3],4,5,6,7,8; no new
> regressions.
> Aurora Perf runs have also been done (DaCapo-h2, Quick Startup/Footprint,
>
On Mon, 9 Nov 2020 23:21:48 GMT, David Holmes wrote:
>> Daniel D. Daugherty has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Resolve more @dholmes-ora comments with help from @fisk.
>
> src/hotspot/share/runtime/synchronizer.cpp line 89:
On Sat, 7 Nov 2020 17:01:42 GMT, Daniel D. Daugherty wrote:
>> src/hotspot/share/runtime/objectMonitor.cpp line 380:
>>
>>> 378: if (event.should_commit()) {
>>> 379: event.set_monitorClass(object()->klass());
>>> 380: event.set_address((uintptr_t)this);
>>
>> This looks wrong - the
On Sat, 7 Nov 2020 17:11:42 GMT, Daniel D. Daugherty wrote:
>> src/hotspot/share/runtime/synchronizer.cpp line 94:
>>
>>> 92: // Find next live ObjectMonitor.
>>> 93: ObjectMonitor* next = m;
>>> 94: while (next != NULL && next->is_being_async_deflated()) {
>>
>> Nit: This
On Sat, 7 Nov 2020 17:22:21 GMT, Daniel D. Daugherty wrote:
>> src/hotspot/share/runtime/synchronizer.cpp line 1520:
>>
>>> 1518: // deflated in this cycle.
>>> 1519: size_t deleted_count = 0;
>>> 1520: for (ObjectMonitor* monitor: delete_list) {
>>
>> I didn't realize C++ has a
On Mon, 9 Nov 2020 15:43:09 GMT, Frederic Parain wrote:
> Please review this small cleanup code, removing the now unused allocation
> type from the fieldInfo structure.
>
> Tested with Mach5, tiers 1 to 3 and locally by running
> test/hotspot/jtreg/serviceability/sa tests.
>
> Thank you,
>
Please review this small cleanup code, removing the now unused allocation type
from the fieldInfo structure.
Tested with Mach5, tiers 1 to 3 and locally by running
test/hotspot/jtreg/serviceability/sa tests.
Thank you,
Fred
-
Commit messages:
- Cleanup fieldInfo structure
35 matches
Mail list logo
