Re: Add array type field for JFR Event

[Denghui Dong]

Hi,
  You are right, but there is an added value "size_safety_cushion" in method ensure_size, 
I think the value of size_safety_cushion should consider the length of char*/jchar* rather than a constant value 1, right?


Thanks,
Denghui Dong
------------------------------------------------------------------
From:Markus Gronlund <markus.gronlund at oracle.com>
Send Time:2019年11月15日(星期五) 21:45
To:董登辉(卓昂) <denghui.ddh at alibaba-inc.com>; hotspot-jfr-dev <hotspot-jfr-dev at openjdk.java.net>; Erik Gahlin <erik.gahlin at oracle.com>
Subject:RE: Add array type field for JFR Event

Hi Denghui,

The writer treats the following types as "strings":

char*
const char*
jstring

They are routed either to write_utf8() or to write_utf16().

Both retrieves the length and serializes the length first as a separate write() invocation and then they send the "array"-part as a separate invocation, write(array, length).
write() and be_write() routes over ensure_size() (or at least should do).

Is there a path that is not covered? If so, can you please give details.

Thanks
Markus



-----Original Message-----
From: Denghui Dong <denghui.ddh at alibaba-inc.com> 
Sent: den 15 november 2019 14:13
To: hotspot-jfr-dev <hotspot-jfr-dev at openjdk.java.net>; erik.gahlin <erik.gahlin at oracle.com>
Subject: Re: Add array type field for JFR Event

Hi Erik,
  Thanks for your reply!
  I agree with you that we should avoid array types in normal cases.
  By the way, when I try to add array type support, I found a problem in implementation of WriterHost<BE, IE, WriterPolicyImpl>::ensure_size(size_t requested), the method doesn't consider the length of the array (should use size_safety_cushion * length).
  Even if there is no array type field, it's still a problem, because string/char* is "array" too.
  What's your comment ?

Best Reards,
Denghui Dong
------------------------------------------------------------------
From:Erik Gahlin <erik.gahlin at oracle.com> Send Time:2019年11月15日(星期五) 20:50 To:hotspot-jfr-dev <hotspot-jfr-dev at openjdk.java.net>
Subject:Re: Add array type field for JFR Event

Hi Denghui,

Previously, in the JRockit version of JFR we had support for arrays type fields in events.

For example, the SystemProperties event had a field with an array of UTFKeyValuePairs. However, they were problematic to consume for clients since you need to index into an array. It is easier if data comes in a normalized form, similar to a relational database where an event corresponds to a row in a table. JMC had to have special handling of these non-normalized events.

When we did the Hotspot port of JFR, we converted events such as SystemProperties into normalized versions and removed the support for array type fields in events. Unless there is a really good use case, I think we should try to avoid them.

Thanks
Erik

On 2019-11-15 11:25, Denghui Dong wrote:
> Hi team,
>    In the current implementation of JFR, it seems that we can't define array type field for JFR events.
>    I have made some changes in GenerateJfrFiles.java so that we can define array type field for native JFR events, it works now.
>    Do any others think it's useful?
>
> Thanks,
> Denghui Dong