RFR (S) 8211239: Build fails without JFR: empty JFR events signatures mismatch

[David Holmes]

Okay I see how this fits together now.

Reviewed.

Thanks,
David

On 1/10/2018 7:59 PM, Aleksey Shipilev wrote:
> On 10/01/2018 11:45 AM, David Holmes wrote:
>> I'm confused. You stated in the RFR:
>>
>> "JFR events generator produces two versions for each event: "full" and "empty", and the version is
>> then selected by INCLUDE_JFR macro during build time."
>>
>> but really it only generates one version and #includes the .hpp with the "real version".
> 
> Note there is JfrEvent superclass, and there are individual subclasses for the actual events. My
> statement is about the actual events, which are generated in two versions:
> 
> $ cat
> ./build/linux-x86_64-server-fastdebug/hotspot/variant-server/gensrc/jfrfiles/jfrEventClasses.hpp |
> nl | grep -C 2 "class EventCompilerStatistics"
>    6825	};
> 
>    6826	class EventCompilerStatistics : public JfrEvent<EventCompilerStatistics>
>    6827	{
>    6828	 private:
> --
>    9896	};
> 
>    9897	class EventCompilerStatistics : public JfrEvent
>    9898	{
>    9899	 public:
> 
> These individual subclasses give us grief right now. The addition of template <typename T> to empty
> JfrEvent is the minor thing that makes both full and empty events agree signature-wise. Granted, the
> JfrEvent superclass can be moved somewhere else for consistency, but that seems to be a more
> intrusive change than this.
> 
>>>> The refactoring to handle the "empty" case within the same logic seems reasonable, though it's not
>>>> immediately obvious to me where the signature mismatch was introduced previously or how the refactor
>>>> prevents that ?
>>>
>>> I think the mismatch between full and empty events was always there, I see it up to the original
>>> commit of JFR event generator. But it was not observed until we started to use the multi-parameter
>>> commit() methods in JDK-8196341: http://hg.openjdk.java.net/jdk/jdk/rev/5f931e3e7a63 -- when it
>>> broke the non-JFR build. The surprises like that is why I think we are better generating the same
>>> methods with/without body on full/empty event paths.
>>
>> This seems error prone to me. I'm guessing we keep far more "stubbed out" JFR code in a non-JFR
>> build than might naively be assumed.
> 
> Automatic generation of same-signature events does look much less error-prone to me, hence the RFR.
> I would not be audacious to claim this is ideal, but at least it solves the current build breakage,
> and makes the whole thing incrementally better.
> 
> -Aleksey
>