Re: ThreadMXBean.getThreadAllocatedBytes() allocates memory

2016-09-19 Thread Bengt Rutisson
Hi Claes and David,

Thanks for the quick responses!

Yes, a pre-allocated array is probably not the way to go. Letting the user
pass a result array is much better or just specialising the "long
getThreadAllocatedBytes(long id)" to avoid  allocations instead of re-using
the "long[] getThreadAllocatedBytes(long[] ids)" version.

As a user of the API I am not surprised that the "long[]
getThreadAllocatedBytes(long[] ids)" version needs to allocate the result
array. But since I know that "long getThreadAllocatedBytes(long id)" pretty
much just needs to read a field in the thread it surprises me that it needs
to allocate.

I'll file an RFE for 10.

Thanks again for the help with this!

Cheers,
Bengt


2016-09-19 1:45 GMT+02:00 David Holmes :

> Hi Bengt,
>
> On 19/09/2016 7:14 AM, Bengt Rutisson wrote:
>
>>
>> Hi Serviceability,
>>
>> Not sure, but I hope this is the correct list to post this on.
>>
>
> Sure is.
>
>
> I wanted to use the ThreadMXBean.getThreadAllocatedBytes() method to get
>> some information about how much memory some Java code allocated.
>>
>> When I dug into the results they didn't properly add up until I realized
>> that the call to getThreadAllocatedBytes() actually allocates memory.
>> This was a surprise to me.
>>
>> I'm attaching a small example to illustrate what I mean.
>>
>> Running the example renders this output:
>>
>> $ javac AllocMeasure.java
>> $ java AllocMeasure
>> Bytes allocated: 48
>> Bytes allocated: 48
>> Bytes allocated: 48
>> Bytes allocated: 48
>> Bytes allocated: 48
>> Bytes allocated: 48
>> Bytes allocated: 48
>> Bytes allocated: 48
>> Bytes allocated: 48
>> Bytes allocated: 48
>>
>> What I would have expected was that it would say "Bytes allocated: 0"
>> since I would like to add my own code between line 9 and 10 in the
>> example and get the value for how much memory it allocates. As it is now
>> I have to deduct the bytes that the getThreadAllocatedBytes() allocates
>> to get the correct result.
>>
>> The problem is that getThreadAllocatedBytes() is implemented this way:
>>
>> public long getThreadAllocatedBytes(long id) {
>> long[] ids = new long[1];
>> ids[0] = id;
>> final long[] sizes = getThreadAllocatedBytes(ids);
>> return sizes[0];
>> }
>>
>> http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/32d957185656/
>> src/java.management/share/classes/sun/management/ThreadImpl.java#l345
>>
>> I was surprised to see the "new long[1]". I realize that it is nice to
>> reuse getThreadAllocatedBytes(long []) method, but maybe a pre-allocated
>> array can be used instead of allocating a new one for each call?
>>
>
> Did you also notice this code:
>
>   protected long[] getThreadAllocatedBytes(long[] ids) {
> boolean verified = verifyThreadAllocatedMemory(ids);
>
> long[] sizes = new long[ids.length];
>
> so we're allocating  another array for the return value(s).
>
> Bit difficult to use a pre-allocated array - when would you allocate it?
> when would you release it? Would you have one per-thread or a freelist of
> them? It all gets a bit too hard to manage.
>
> A better API would allow the user to pass in the result array as well - to
> allow for array allocation and reuse outside of the region of code that is
> being measured.
>
> I know the specification for this method is kind of fuzzy, but is this
>> to be considered a bug or does it work as intended?
>>
>
> I'd call it a quality of implementation issue. It would be better if the
> allocation could be avoided, but that requires two entry points to the VM
> code. Certainly the query for a single thread should avoid the need for any
> array allocation. But I think this pattern is common throughout the
> management code.
>
> You should a file a RFE for 10.
>
> Cheers,
> David
>
>
> Thanks,
>> Bengt
>>
>


Re: ThreadMXBean.getThreadAllocatedBytes() allocates memory

2016-09-18 Thread David Holmes

Hi Bengt,

On 19/09/2016 7:14 AM, Bengt Rutisson wrote:


Hi Serviceability,

Not sure, but I hope this is the correct list to post this on.


Sure is.


I wanted to use the ThreadMXBean.getThreadAllocatedBytes() method to get
some information about how much memory some Java code allocated.

When I dug into the results they didn't properly add up until I realized
that the call to getThreadAllocatedBytes() actually allocates memory.
This was a surprise to me.

I'm attaching a small example to illustrate what I mean.

Running the example renders this output:

$ javac AllocMeasure.java
$ java AllocMeasure
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48

What I would have expected was that it would say "Bytes allocated: 0"
since I would like to add my own code between line 9 and 10 in the
example and get the value for how much memory it allocates. As it is now
I have to deduct the bytes that the getThreadAllocatedBytes() allocates
to get the correct result.

The problem is that getThreadAllocatedBytes() is implemented this way:

public long getThreadAllocatedBytes(long id) {
long[] ids = new long[1];
ids[0] = id;
final long[] sizes = getThreadAllocatedBytes(ids);
return sizes[0];
}

http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/32d957185656/src/java.management/share/classes/sun/management/ThreadImpl.java#l345

I was surprised to see the "new long[1]". I realize that it is nice to
reuse getThreadAllocatedBytes(long []) method, but maybe a pre-allocated
array can be used instead of allocating a new one for each call?


Did you also notice this code:

  protected long[] getThreadAllocatedBytes(long[] ids) {
boolean verified = verifyThreadAllocatedMemory(ids);

long[] sizes = new long[ids.length];

so we're allocating  another array for the return value(s).

Bit difficult to use a pre-allocated array - when would you allocate it? 
when would you release it? Would you have one per-thread or a freelist 
of them? It all gets a bit too hard to manage.


A better API would allow the user to pass in the result array as well - 
to allow for array allocation and reuse outside of the region of code 
that is being measured.



I know the specification for this method is kind of fuzzy, but is this
to be considered a bug or does it work as intended?


I'd call it a quality of implementation issue. It would be better if the 
allocation could be avoided, but that requires two entry points to the 
VM code. Certainly the query for a single thread should avoid the need 
for any array allocation. But I think this pattern is common throughout 
the management code.


You should a file a RFE for 10.

Cheers,
David



Thanks,
Bengt


Re: ThreadMXBean.getThreadAllocatedBytes() allocates memory

2016-09-18 Thread Claes Redestad

Hi Bengt,

I'm not Serviceability, but you know I can't leave them micro-
optimizations alone! :-)

So, reusing cached arrays could be made to work but would require
some synchronization to keep things thread-safe and tidy[1].

This will complicate the code, especially since there's another implied
allocation in getThreadAllocatedBytes. Not to mention that caching
objects which are cheap to allocate is a bit of an performance
anti-pattern.

Adding synchronization also comes with it's own risks, especially as
we're calling into JNI and the VM code takes a somewhat shady mutex
already (Threads_lock).

Generally I don't think there's ever any behavioral guarantees about how
much - or little - a  method won't allocate anything, so calling this a
bug is a bit of a stretch IMO, although it's a bit unfortunate in this
particular case.

TL;DR: I'm a bit skeptic, but if it's important to you to fix this, I
wouldn't think it's impossible.

Thanks!

/Claes

[1] Alternatively we could of course implement a JNI method taking a
long rather than a long[], which would be consistent with other methods
in ThreadImpl.java, but I think we want to avoid going that far.

On 2016-09-18 23:14, Bengt Rutisson wrote:


Hi Serviceability,

Not sure, but I hope this is the correct list to post this on.

I wanted to use the ThreadMXBean.getThreadAllocatedBytes() method to get
some information about how much memory some Java code allocated.

When I dug into the results they didn't properly add up until I realized
that the call to getThreadAllocatedBytes() actually allocates memory.
This was a surprise to me.

I'm attaching a small example to illustrate what I mean.

Running the example renders this output:

$ javac AllocMeasure.java
$ java AllocMeasure
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48

What I would have expected was that it would say "Bytes allocated: 0"
since I would like to add my own code between line 9 and 10 in the
example and get the value for how much memory it allocates. As it is now
I have to deduct the bytes that the getThreadAllocatedBytes() allocates
to get the correct result.

The problem is that getThreadAllocatedBytes() is implemented this way:

 public long getThreadAllocatedBytes(long id) {
 long[] ids = new long[1];
 ids[0] = id;
 final long[] sizes = getThreadAllocatedBytes(ids);
 return sizes[0];
 }

http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/32d957185656/src/java.management/share/classes/sun/management/ThreadImpl.java#l345

I was surprised to see the "new long[1]". I realize that it is nice to
reuse getThreadAllocatedBytes(long []) method, but maybe a pre-allocated
array can be used instead of allocating a new one for each call?

I know the specification for this method is kind of fuzzy, but is this
to be considered a bug or does it work as intended?

Thanks,
Bengt


ThreadMXBean.getThreadAllocatedBytes() allocates memory

2016-09-18 Thread Bengt Rutisson
Hi Serviceability,

Not sure, but I hope this is the correct list to post this on.

I wanted to use the ThreadMXBean.getThreadAllocatedBytes() method to get
some information about how much memory some Java code allocated.

When I dug into the results they didn't properly add up until I realized
that the call to getThreadAllocatedBytes() actually allocates memory. This
was a surprise to me.

I'm attaching a small example to illustrate what I mean.

Running the example renders this output:

$ javac AllocMeasure.java
$ java AllocMeasure
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48
Bytes allocated: 48

What I would have expected was that it would say "Bytes allocated: 0" since
I would like to add my own code between line 9 and 10 in the example and
get the value for how much memory it allocates. As it is now I have to
deduct the bytes that the getThreadAllocatedBytes() allocates to get the
correct result.

The problem is that getThreadAllocatedBytes() is implemented this way:

public long getThreadAllocatedBytes(long id) {
long[] ids = new long[1];
ids[0] = id;
final long[] sizes = getThreadAllocatedBytes(ids);
return sizes[0];
}

http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/32d957185656/src/java.management/share/classes/sun/management/ThreadImpl.java#l345

I was surprised to see the "new long[1]". I realize that it is nice to
reuse getThreadAllocatedBytes(long []) method, but maybe a pre-allocated
array can be used instead of allocating a new one for each call?

I know the specification for this method is kind of fuzzy, but is this to
be considered a bug or does it work as intended?

Thanks,
Bengt


AllocMeasure.java
Description: Binary data