8246260: JFR: Write event size field without padding

Reviewed-by: jbachorik, mgronlun

make/src/classes/build/tools/jfr/GenerateJfrFiles.java

@@ -404,7 +404,8 @@ private static void printJfrEventControlHpp(Metadata metadata, TypeCounter typeC
             out.write("  jlong  cutoff_ticks;");
             out.write("  u1     stacktrace;");
             out.write("  u1     enabled;");
-            out.write("  u1     pad[6]; // Because GCC on linux ia32 at least tries to pack this.");
+            out.write("  u1     large;");
+            out.write("  u1     pad[5]; // Because GCC on linux ia32 at least tries to pack this.");
             out.write("};");
             out.write("");
             out.write("union JfrNativeSettings {");

src/hotspot/share/jfr/recorder/jfrEventSetting.cpp

@@ -53,6 +53,11 @@ void JfrEventSetting::set_enabled(jlong id, bool enabled) {
   setting(event_id).enabled = enabled;
 }
 
+void JfrEventSetting::set_large(JfrEventId event_id) {
+  assert(bounds_check_event(event_id), "invariant");
+  setting(event_id).large = true;
+}
+
 #ifdef ASSERT
 bool JfrEventSetting::bounds_check_event(jlong id) {
   if ((unsigned)id < FIRST_EVENT_ID) {

src/hotspot/share/jfr/recorder/jfrEventSetting.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -46,6 +46,9 @@ class JfrEventSetting : AllStatic {
   static jlong threshold(JfrEventId event_id);
   static bool set_cutoff(jlong event_id, jlong cutoff_ticks);
   static jlong cutoff(JfrEventId event_id);
+  static bool is_large(JfrEventId event_id);
+  static void set_large(JfrEventId event_id);
+
   DEBUG_ONLY(static bool bounds_check_event(jlong id);)
 };
 

src/hotspot/share/jfr/recorder/jfrEventSetting.inline.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -47,4 +47,7 @@ inline jlong JfrEventSetting::cutoff(JfrEventId event_id) {
   return setting(event_id).cutoff_ticks;
 }
 
+inline bool JfrEventSetting::is_large(JfrEventId event_id) {
+  return setting(event_id).large != 0;
+}
 #endif // SHARE_JFR_RECORDER_JFREVENTSETTING_INLINE_HPP

src/hotspot/share/jfr/recorder/service/jfrEvent.hpp

@@ -119,6 +119,14 @@ class JfrEvent {
     return JfrEventSetting::has_stacktrace(T::eventId);
   }
 
+  static bool is_large() {
+    return JfrEventSetting::is_large(T::eventId);
+  }
+
+  static void set_large() {
+    JfrEventSetting::set_large(T::eventId);
+  }
+
   static JfrEventId id() {
     return T::eventId;
   }
@@ -160,7 +168,23 @@ class JfrEvent {
       // most likely a pending OOM
       return;
     }
+    bool large = is_large();
+    if (write_sized_event(buffer, event_thread, tl, large)) {
+      // Event written succesfully
+      return;
+    }
+    if (!large) {
+      // Try large size
+      if (write_sized_event(buffer, event_thread, tl, true)) {
+        // Event written succesfully, use large size from now on
+        set_large();
+      }
+    }
+  }
+
+  bool write_sized_event(JfrBuffer* const buffer, Thread* const event_thread, JfrThreadLocal* const tl, bool large_size) {
     JfrNativeEventWriter writer(buffer, event_thread);
+    writer.begin_event_write(large_size);
     writer.write<u8>(T::eventId);
     assert(_start_time != 0, "invariant");
     writer.write(_start_time);
@@ -184,6 +208,7 @@ class JfrEvent {
     }
     // payload
     static_cast<T*>(this)->writeData(writer);
+    return writer.end_event_write(large_size) > 0;
   }
 
 #ifdef ASSERT

src/hotspot/share/jfr/recorder/storage/jfrStorage.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -224,10 +224,12 @@ static void write_data_loss_event(JfrBuffer* buffer, u8 unflushed_size, Thread*
   const u8 total_data_loss = thread->jfr_thread_local()->add_data_lost(unflushed_size);
   if (EventDataLoss::is_enabled()) {
     JfrNativeEventWriter writer(buffer, thread);
+    writer.begin_event_write(false);
     writer.write<u8>(EventDataLoss::eventId);
     writer.write(JfrTicks::now());
     writer.write(unflushed_size);
     writer.write(total_data_loss);
+    writer.end_event_write(false);
   }
 }
 

src/hotspot/share/jfr/writers/jfrEventWriterHost.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -35,17 +35,8 @@ class EventWriterHost : public WriterHost<BE, IE, WriterPolicyImpl> {
   EventWriterHost(Thread* thread);
   void begin_write();
   intptr_t end_write();
-  void begin_event_write();
-  intptr_t end_event_write();
-};
-
-template <typename BE, typename IE, typename WriterPolicyImpl >
-class StackEventWriterHost : public EventWriterHost<BE, IE, WriterPolicyImpl> {
- public:
-  template <typename StorageType>
-  StackEventWriterHost(StorageType* storage, Thread* thread);
-  StackEventWriterHost(Thread* thread);
-  ~StackEventWriterHost();
+  void begin_event_write(bool large);
+  intptr_t end_event_write(bool large);
 };
 
 #endif // SHARE_JFR_WRITERS_JFREVENTWRITERHOST_HPP

src/hotspot/share/jfr/writers/jfrEventWriterHost.inline.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -33,7 +33,7 @@ inline EventWriterHost<BE, IE, WriterPolicyImpl>::
 EventWriterHost(StorageType* storage, Thread* thread) : WriterHost<BE, IE, WriterPolicyImpl>(storage, thread) {}
 
 template <typename BE, typename IE, typename WriterPolicyImpl>
-inline EventWriterHost<BE, IE, WriterPolicyImpl>::EventWriterHost(Thread* thread) : WriterHost<BE, IE, WriterPolicyImpl>(thread) {
+inline EventWriterHost<BE, IE, WriterPolicyImpl>::EventWriterHost(Thread* thread)  : WriterHost<BE, IE, WriterPolicyImpl>(thread) {
 }
 
 template <typename BE, typename IE, typename WriterPolicyImpl>
@@ -53,45 +53,47 @@ inline intptr_t EventWriterHost<BE, IE, WriterPolicyImpl>::end_write(void) {
 }
 
 template <typename BE, typename IE, typename WriterPolicyImpl>
-inline void EventWriterHost<BE, IE, WriterPolicyImpl>::begin_event_write() {
+inline void EventWriterHost<BE, IE, WriterPolicyImpl>::begin_event_write(bool large) {
   assert(this->is_valid(), "invariant");
   assert(!this->is_acquired(), "calling begin with writer already in acquired state!");
   this->begin_write();
-  this->reserve(sizeof(u4)); // reserve the event size slot
+  // reserve the event size slot
+  if (large) {
+    this->reserve(sizeof(u4));
+  } else {
+    this->reserve(sizeof(u1));
+  }
 }
 
 template <typename BE, typename IE, typename WriterPolicyImpl>
-inline intptr_t EventWriterHost<BE, IE, WriterPolicyImpl>::end_event_write() {
+inline intptr_t EventWriterHost<BE, IE, WriterPolicyImpl>::end_event_write(bool large) {
   assert(this->is_acquired(), "invariant");
   if (!this->is_valid()) {
     this->release();
     return 0;
   }
-  const u4 written = (u4)end_write();
-  if (written > sizeof(u4)) { // larger than header reserve
-    this->write_padded_at_offset(written, 0);
-    this->commit();
+  u4 written = (u4)end_write();
+  if (large) {
+    // size written is larger than header reserve, so commit
+    if (written > sizeof(u4)) {
+      this->write_padded_at_offset(written, 0);
+      this->commit();
+    }
+  } else {
+    // abort if event size will not fit in one byte (compressed)
+    if (written > 127) {
+      this->reset();
+      written = 0;
+    } else {
+      // size written is larger than header reserve, so commit
+      if (written > sizeof(u1)) {
+        this->write_at_offset(written, 0);
+        this->commit();
+      }
+    }
   }
   this->release();
   assert(!this->is_acquired(), "invariant");
   return written;
 }
-
-template <typename BE, typename IE, typename WriterPolicyImpl>
-template <typename StorageType>
-inline StackEventWriterHost<BE, IE, WriterPolicyImpl>::
-StackEventWriterHost(StorageType* storage, Thread* thread) : EventWriterHost<BE, IE, WriterPolicyImpl>(storage, thread) {
-  this->begin_event_write();
-}
-
-template <typename BE, typename IE, typename WriterPolicyImpl>
-inline StackEventWriterHost<BE, IE, WriterPolicyImpl>::StackEventWriterHost(Thread* thread) : EventWriterHost<BE, IE, WriterPolicyImpl>(thread) {
-  this->begin_event_write();
-}
-
-template <typename BE, typename IE, typename WriterPolicyImpl>
-inline StackEventWriterHost<BE, IE, WriterPolicyImpl>::~StackEventWriterHost() {
-  this->end_event_write();
-}
-
 #endif // SHARE_JFR_WRITERS_JFREVENTWRITERHOST_INLINE_HPP

src/hotspot/share/jfr/writers/jfrJavaEventWriter.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -44,7 +44,6 @@ static int start_pos_offset = invalid_offset;
 static int start_pos_address_offset = invalid_offset;
 static int current_pos_offset = invalid_offset;
 static int max_pos_offset = invalid_offset;
-static int max_event_size_offset = invalid_offset;
 static int notified_offset = invalid_offset;
 static int thread_id_offset = invalid_offset;
 static int valid_offset = invalid_offset;
@@ -110,13 +109,6 @@ static bool setup_event_writer_offsets(TRAPS) {
   compute_offset(max_pos_offset, klass, max_pos_sym, vmSymbols::long_signature());
   assert(max_pos_offset != invalid_offset, "invariant");
 
-  const char max_event_size_name[] = "maxEventSize";
-  Symbol* const max_event_size_sym = SymbolTable::new_symbol(max_event_size_name);
-  assert (max_event_size_sym != NULL, "invariant");
-  assert(invalid_offset == max_event_size_offset, "invariant");
-  compute_offset(max_event_size_offset, klass, max_event_size_sym, vmSymbols::int_signature());
-  assert(max_event_size_offset != invalid_offset, "invariant");
-
   const char notified_name[] = "notified";
   Symbol* const notified_sym = SymbolTable::new_symbol(notified_name);
   assert (notified_sym != NULL, "invariant");

src/hotspot/share/jfr/writers/jfrNativeEventWriter.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -33,6 +33,6 @@
 
 typedef Adapter<JfrFlush> JfrNativeEventAdapter;
 typedef MemoryWriterHost<JfrNativeEventAdapter, StackObj> JfrNativeEventWriterImpl;
-typedef StackEventWriterHost<BigEndianEncoder, CompressedIntegerEncoder, JfrNativeEventWriterImpl> JfrNativeEventWriter;
+typedef EventWriterHost<BigEndianEncoder, CompressedIntegerEncoder, JfrNativeEventWriterImpl> JfrNativeEventWriter;
 
 #endif // SHARE_JFR_WRITERS_JFRNATIVEEVENTWRITER_HPP

src/jdk.jfr/share/classes/jdk/jfr/internal/EventWriter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -35,21 +35,25 @@
  *
  */
 public final class EventWriter {
+
+    // Event may not exceed size for a padded integer
+    private static final long MAX_EVENT_SIZE = (1 << 28) -1;
     private static final Unsafe unsafe = Unsafe.getUnsafe();
     private final static JVM jvm = JVM.getJVM();
 
+    // The JVM needs access to these values. Don't remove
+    private final long threadID;
     private long startPosition;
     private long startPositionAddress;
     private long currentPosition;
     private long maxPosition;
-    private final long threadID;
+    private boolean valid;
+    boolean notified; // Not private to avoid being optimized away
+
     private PlatformEventType eventType;
-    private int maxEventSize;
     private boolean started;
-    private boolean valid;
     private boolean flushOnEnd;
-    // set by the JVM, not private to avoid being optimized out
-    boolean notified;
+    private boolean largeSize = false;
 
     public static EventWriter getEventWriter() {
         EventWriter ew = (EventWriter)JVM.getEventWriter();
@@ -175,9 +179,15 @@ public void putStackTrace() {
     }
 
     private void reserveEventSizeField() {
-        // move currentPosition Integer.Bytes offset from start position
-        if (isValidForSize(Integer.BYTES)) {
-            currentPosition += Integer.BYTES;
+        this.largeSize = eventType.isLargeSize();
+        if (largeSize) {
+            if (isValidForSize(Integer.BYTES)) {
+                currentPosition +=  Integer.BYTES;
+            }
+        } else {
+            if (isValidForSize(1)) {
+                currentPosition += 1;
+            }
         }
     }
 
@@ -242,11 +252,25 @@ public boolean endEvent() {
             return true;
         }
         final int eventSize = usedSize();
-        if (eventSize > maxEventSize) {
+        if (eventSize > MAX_EVENT_SIZE) {
             reset();
             return true;
         }
-        Bits.putInt(startPosition, makePaddedInt(eventSize));
+
+        if (largeSize) {
+            Bits.putInt(startPosition, makePaddedInt(eventSize));
+        } else {
+            if (eventSize < 128) {
+                Bits.putByte(startPosition, (byte) eventSize);
+            } else {
+                eventType.setLargeSize();
+                reset();
+                // returning false will trigger restart of the
+                // event write attempt
+                return false;
+            }
+        }
+
         if (isNotified()) {
             resetNotified();
             reset();
@@ -273,8 +297,6 @@ private EventWriter(long startPos, long maxPos, long startPosAddress, long threa
         flushOnEnd = false;
         this.valid = valid;
         notified = false;
-        // event may not exceed size for a padded integer
-        maxEventSize = (1 << 28) -1;
     }
 
     private static int makePaddedInt(int v) {

src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformEventType.java

@@ -46,6 +46,7 @@ public final class PlatformEventType extends Type {
     private final int stackTraceOffset;
 
     // default values
+    private boolean largeSize = false;
     private boolean enabled = false;
     private boolean stackTraceEnabled = true;
     private long thresholdTicks = 0;
@@ -278,4 +279,12 @@ public boolean isCommittable() {
     public int getStackTraceOffset() {
         return stackTraceOffset;
     }
+
+    public boolean isLargeSize() {
+        return largeSize;
+    }
+
+    public void setLargeSize() {
+        largeSize = true;
+    }
 }