Hi Roman,
I'm looking at the x86 code. I noticed that We used to always call
incr_allocated_bytes after calling eden_allocate. However, now you moved
incr_allocated_bytes into eden_allocate, which makes sense, and I like
that. But it is only incremented if inline contig alloc is supported,
otherwise it calls the slowpath without incrementing allocation bytes.
So that is seemingly changing the behaviour from before. Is that a bug
in your code, a bug in the current code, or am I misunderstanding this
code? It looks to me that your code is fixing a bug in the case when not
using TLAB and not using contig allocation. Today it seems to me that we
will always call the slowpath that always increments allocated bytes,
and then always account it one more time in the interpreter. There might
be more subtle differences as well that I have not considered yet. Thoughts?
Otherwise, I think this looks reasonable.
Thanks,
/Erik
Other than that, it looks reasonable.
On 2018-06-19 21:46, Roman Kennke wrote:
> Am 19.06.2018 um 19:56 schrieb Andrew Haley:
>> On 06/19/2018 06:14 PM, Roman Kennke wrote:
>>> Can I please get reviews?
>> AArch64 looks OK, but it makes no sense for Register thread to be an
>> argument to eden_allocate: it's rthread. Otherwise fine.
>>>> Unless you've messed up copying some of the code, but it'd be hard
>> to check all that by hand without half a day to spare... :-(
> Right. It was pre-existing (probably modeled after x86), and I wanted to
> make it a 1:1 copy/refactoring job as much as possible, but yeah this is
> pointless in aarch64. Let's remove it:
>> Incremental:
>http://cr.openjdk.java.net/~rkennke/JDK-8205336/webrev.01.diff/> Full:
>http://cr.openjdk.java.net/~rkennke/JDK-8205336/webrev.01/>> Good now?
>> Thanks for reviewing!
> Roman
>