RFR: 8027545: Improve object array chunking test in G1's copy_to_survivor_space [v2]
Kim Barrett
kim.barrett at oracle.com
Sat Sep 12 01:02:10 UTC 2020
> On Sep 11, 2020, at 8:50 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>
> Hi,
>
> On 11.09.20 11:01, Kim Barrett wrote:
> [...]
>> Kim Barrett has updated the pull request incrementally with one additional commit since the last revision:
>> Respond to reviews by ayang and sjohanss.
> >
>> Changes:
>> - all: https://git.openjdk.java.net/jdk/pull/90/files
>> - new: https://git.openjdk.java.net/jdk/pull/90/files/9397514e..1981c392
>> Webrevs:
>> - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=90&range=01
>> - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=90&range=00-01
>> Stats: 152 lines in 6 files changed: 122 ins; 14 del; 16 mod
>> Patch: https://git.openjdk.java.net/jdk/pull/90.diff
>> Fetch: git fetch https://git.openjdk.java.net/jdk pull/90/head:pull/90
>> PR: https://git.openjdk.java.net/jdk/pull/90
>
> - partialArrayTaskStepper.hpp:
>
> s/_task_fannout/_task_fanout (or maybe _task_fan_out)
I thought it looked a little odd at first, but never got around to spell
checking it, and after a while it looked fine by familiarity. Will fix
("fanout").
> - since _task_fannout and _task_limit are only assigned in the constructor of PartialArrayTaskStepper, maybe make them const?
I'd rather not. I've been down that road and didn't like the results. Just
as an example, doing that sort of thing renders a class copyable but not
assignable, swappable, or (in some cases) moveable from. That's an odd
combination. I already (though recently) made start() and next() const.
> - also the asserts in PartialArrayTaskStepper::next_impl could be moved to the constructor initialization list but otoh the fannout > 0 assert does make the calculation below immediately obviously safe.
As you say, the current location shows the requirements for the nearby
calculations.
> Looks good otherwise.
Thanks.
> Thanks,
> Thomas
More information about the hotspot-dev
mailing list