8246260: JFR: Write event size field without padding

Reviewed-by: jbachorik, mgronlun

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

@@ -404,7 +404,8 @@ public class GenerateJfrFiles {
             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();
+  void begin_event_write(bool large);
-  intptr_t end_event_write();
+  intptr_t end_event_write(bool large);
-};
-
-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();
 };
 
 #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();
+  u4 written = (u4)end_write();
-  if (written > sizeof(u4)) { // larger than header reserve
+  if (large) {
-    this->write_padded_at_offset(written, 0);
+    // size written is larger than header reserve, so commit
-    this->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 @@ import jdk.jfr.internal.consumer.StringParser;
  *
  */
 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 PlatformEventType eventType;
-    private int maxEventSize;
-    private boolean started;
     private boolean valid;
+    boolean notified; // Not private to avoid being optimized away
+
+    private PlatformEventType eventType;
+    private boolean started;
     private boolean flushOnEnd;
-    // set by the JVM, not private to avoid being optimized out
+    private boolean largeSize = false;
-    boolean notified;
 
     public static EventWriter getEventWriter() {
         EventWriter ew = (EventWriter)JVM.getEventWriter();
@@ -175,9 +179,15 @@ public final class EventWriter {
     }
 
     private void reserveEventSizeField() {
-        // move currentPosition Integer.Bytes offset from start position
+        this.largeSize = eventType.isLargeSize();
-        if (isValidForSize(Integer.BYTES)) {
+        if (largeSize) {
-            currentPosition += Integer.BYTES;
+            if (isValidForSize(Integer.BYTES)) {
+                currentPosition +=  Integer.BYTES;
+            }
+        } else {
+            if (isValidForSize(1)) {
+                currentPosition += 1;
+            }
         }
     }
 
@@ -242,11 +252,25 @@ public final class EventWriter {
             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 @@ public final class EventWriter {
         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 final class PlatformEventType extends Type {
     public int getStackTraceOffset() {
         return stackTraceOffset;
     }
+
+    public boolean isLargeSize() {
+        return largeSize;
+    }
+
+    public void setLargeSize() {
+        largeSize = true;
+    }
 }

test/jdk/jdk/jfr/jvm/TestFatEvent.java

@@ -0,0 +1,136 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package jdk.jfr.jvm;
+
+import java.io.IOException;
+import java.util.List;
+
+import jdk.jfr.Event;
+import jdk.jfr.Recording;
+import jdk.jfr.ValueDescriptor;
+import jdk.jfr.consumer.RecordedEvent;
+import jdk.test.lib.Asserts;
+import jdk.test.lib.jfr.EventNames;
+import jdk.test.lib.jfr.Events;
+
+/**
+ * @test TestFatEvent
+ * @key jfr
+ * @requires vm.hasJFR
+ * @library /test/lib
+ * @run main/othervm -Dprop1=12345678901234567890123456789012345678901234567890
+ *      -Dprop2=12345678901234567890123456789012345678901234567890
+ *      -Dprop3=12345678901234567890123456789012345678901234567890
+ *      jdk.jfr.jvm.TestFatEvent
+ */
+public class TestFatEvent {
+
+    public static void main(String... args) throws Exception {
+        testFatNativeEvent();
+        testFatJavaEvent();
+    }
+
+    private static void testFatNativeEvent() throws Exception {
+        try (Recording r = new Recording()) {
+            r.enable(EventNames.JVMInformation).with("period", "everyChunk");
+            r.start();
+            r.stop();
+            List<RecordedEvent> events = Events.fromRecording(r);
+            Asserts.assertEquals(2, events.size());
+            for (RecordedEvent e : events) {
+                String s = e.getString("jvmArguments");
+                if (s.length() < 150) {
+                    throw new Exception("Expected at least 150 characters");
+                }
+            }
+        }
+    }
+    private static final Long expected = Long.MAX_VALUE;
+
+    private static void testFatJavaEvent() throws IOException, Exception {
+        // This event use more than 127 bytes
+        // which requires two bytes in compressed
+        // integer format
+        class FatEvent extends Event {
+            long a = expected;
+            long b = expected;
+            long c = expected;
+            long d = expected;
+            long e = expected;
+            long f = expected;
+            long g = expected;
+            long h = expected;
+            long i = expected;
+            long j = expected;
+            long k = expected;
+            long l = expected;
+            long m = expected;
+            long n = expected;
+            long o = expected;
+            long p = expected;
+            long q = expected;
+            long r = expected;
+            long s = expected;
+            long t = expected;
+            long u = expected;
+            long v = expected;
+            long w = expected;
+            long x = expected;
+            long y = expected;
+            long z = expected;
+        }
+        try (Recording r = new Recording()) {
+            r.start();
+            int eventCount = 5000; //
+            for (int i = 0; i < eventCount; i++) {
+                FatEvent event = new FatEvent();
+                event.commit();
+            }
+            r.stop();
+            List<RecordedEvent> events = Events.fromRecording(r);
+            int count = 0;
+            for (RecordedEvent event : events) {
+                verifyEvent(event);
+                count++;
+            }
+            if (count != eventCount) {
+                throw new Exception("Unexpected event count " + count + ", expected " + eventCount);
+            }
+        }
+    }
+
+    private static void verifyEvent(RecordedEvent e) throws Exception {
+        for (ValueDescriptor v : e.getEventType().getFields()) {
+            String fieldName = v.getName();
+            Object o = e.getValue(v.getName());
+            if (fieldName.length() == 1 && !o.equals(expected)) {
+                System.out.println("Expected: " + expected);
+                System.out.println(e);
+                throw new Exception("Unexpected value " + o + " for field " + fieldName);
+            }
+        }
+    }
+}