Request for Comments: Backport of JDK-8166317: InterpreterCodeSize should be computed

Leslie Zhai zhaixiang at loongson.cn
Mon Sep 17 08:05:21 UTC 2018


Hi David,

Thanks for your suggestion!

Before I know how to get someone to host the patch on 
cr.openjdk.java.net, I will paste it here first :)

Thanks,

Leslie Zhai

----- 8< -------- 8< -------- 8< -------- 8< -------- 8< -------- 8< ---

diff -r 27d48c01c0cf src/cpu/sparc/vm/macroAssembler_sparc.cpp
--- a/src/cpu/sparc/vm/macroAssembler_sparc.cpp    Fri Sep 14 16:39:43 
2018 +0800
+++ b/src/cpu/sparc/vm/macroAssembler_sparc.cpp    Mon Sep 17 10:57:24 
2018 +0800
@@ -3642,32 +3642,6 @@
  #undef __
  }

-static inline void generate_satb_log_enqueue_if_necessary(bool 
with_frame) {
-  if (with_frame) {
-    if (satb_log_enqueue_with_frame == 0) {
-      generate_satb_log_enqueue(with_frame);
-      assert(satb_log_enqueue_with_frame != 0, "postcondition.");
-      if (G1SATBPrintStubs) {
-        tty->print_cr("Generated with-frame satb enqueue:");
- Disassembler::decode((u_char*)satb_log_enqueue_with_frame,
-                             satb_log_enqueue_with_frame_end,
-                             tty);
-      }
-    }
-  } else {
-    if (satb_log_enqueue_frameless == 0) {
-      generate_satb_log_enqueue(with_frame);
-      assert(satb_log_enqueue_frameless != 0, "postcondition.");
-      if (G1SATBPrintStubs) {
-        tty->print_cr("Generated frameless satb enqueue:");
-        Disassembler::decode((u_char*)satb_log_enqueue_frameless,
-                             satb_log_enqueue_frameless_end,
-                             tty);
-      }
-    }
-  }
-}
-
  void MacroAssembler::g1_write_barrier_pre(Register obj,
                                            Register index,
                                            int offset,
@@ -3737,13 +3711,9 @@
              "Or we need to think harder.");

    if (pre_val->is_global() && !preserve_o_regs) {
-    generate_satb_log_enqueue_if_necessary(true); // with frame
-
      call(satb_log_enqueue_with_frame);
      delayed()->mov(pre_val, O0);
    } else {
-    generate_satb_log_enqueue_if_necessary(false); // frameless
-
      save_frame(0);
      call(satb_log_enqueue_frameless);
      delayed()->mov(pre_val->after_save(), O0);
@@ -3851,20 +3821,6 @@

  }

-static inline void
-generate_dirty_card_log_enqueue_if_necessary(jbyte* byte_map_base) {
-  if (dirty_card_log_enqueue == 0) {
-    generate_dirty_card_log_enqueue(byte_map_base);
-    assert(dirty_card_log_enqueue != 0, "postcondition.");
-    if (G1SATBPrintStubs) {
-      tty->print_cr("Generated dirty_card enqueue:");
-      Disassembler::decode((u_char*)dirty_card_log_enqueue,
-                           dirty_card_log_enqueue_end,
-                           tty);
-    }
-  }
-}
-

  void MacroAssembler::g1_write_barrier_post(Register store_addr, 
Register new_val, Register tmp) {

@@ -3900,7 +3856,6 @@
    } else {
      post_filter_masm->nop();
    }
- generate_dirty_card_log_enqueue_if_necessary(bs->byte_map_base);
    save_frame(0);
    call(dirty_card_log_enqueue);
    if (use_scr) {
@@ -3913,6 +3868,28 @@
    bind(filtered);
  }

+// Called from init_globals() after universe_init() and before 
interpreter_init()
+void g1_barrier_stubs_init() {
+  CollectedHeap* heap = Universe::heap();
+  if (heap->kind() == CollectedHeap::G1CollectedHeap) {
+    // Only needed for G1
+    if (dirty_card_log_enqueue == 0) {
+      G1SATBCardTableLoggingModRefBS* bs =
+ barrier_set_cast<G1SATBCardTableLoggingModRefBS>(heap->barrier_set());
+      generate_dirty_card_log_enqueue(bs->byte_map_base);
+      assert(dirty_card_log_enqueue != 0, "postcondition.");
+    }
+    if (satb_log_enqueue_with_frame == 0) {
+      generate_satb_log_enqueue(true);
+      assert(satb_log_enqueue_with_frame != 0, "postcondition.");
+    }
+    if (satb_log_enqueue_frameless == 0) {
+      generate_satb_log_enqueue(false);
+      assert(satb_log_enqueue_frameless != 0, "postcondition.");
+    }
+  }
+}
+
  #endif // INCLUDE_ALL_GCS
  ///////////////////////////////////////////////////////////////////////////////////

diff -r 27d48c01c0cf src/share/vm/code/codeBlob.cpp
--- a/src/share/vm/code/codeBlob.cpp    Fri Sep 14 16:39:43 2018 +0800
+++ b/src/share/vm/code/codeBlob.cpp    Mon Sep 17 10:57:24 2018 +0800
@@ -67,7 +67,7 @@
  #include "c1/c1_Runtime1.hpp"
  #endif

-unsigned int align_code_offset(int offset) {
+unsigned int CodeBlob::align_code_offset(int offset) {
    // align the size to CodeEntryAlignment
    return
      ((offset + (int)CodeHeap::header_size() + (CodeEntryAlignment-1)) 
& ~(CodeEntryAlignment-1))
diff -r 27d48c01c0cf src/share/vm/code/codeBlob.hpp
--- a/src/share/vm/code/codeBlob.hpp    Fri Sep 14 16:39:43 2018 +0800
+++ b/src/share/vm/code/codeBlob.hpp    Mon Sep 17 10:57:24 2018 +0800
@@ -71,6 +71,7 @@
   public:
    // Returns the space needed for CodeBlob
    static unsigned int allocation_size(CodeBuffer* cb, int header_size);
+  static unsigned int align_code_offset(int offset);

    // Creation
    // a) simple CodeBlob
@@ -135,6 +136,11 @@
    int content_size() const                       { return           
content_end()    -           content_begin(); }
    int code_size() const                          { return           
code_end()       -           code_begin(); }
    int data_size() const                          { return           
data_end()       -           data_begin(); }
+  // Only used from CodeCache::free_unused_tail() after the Interpreter 
blob was trimmed
+  void adjust_size(size_t used) {
+    _size = (int)used;
+    _data_offset = (int)used;
+  }

    // Containment
    bool blob_contains(address addr) const         { return 
header_begin()       <= addr && addr < data_end();       }
diff -r 27d48c01c0cf src/share/vm/code/codeCache.cpp
--- a/src/share/vm/code/codeCache.cpp    Fri Sep 14 16:39:43 2018 +0800
+++ b/src/share/vm/code/codeCache.cpp    Mon Sep 17 10:57:24 2018 +0800
@@ -227,6 +227,23 @@
  }


+void CodeCache::free_unused_tail(CodeBlob* cb, size_t used) {
+  assert_locked_or_safepoint(CodeCache_lock);
+  guarantee(cb->is_buffer_blob() && strncmp("Interpreter", cb->name(), 
11) == 0, "Only possible for interpreter!");
+  print_trace("free_unused_tail", cb);
+
+  // We also have to account for the extra space (i.e. header) used by 
the CodeBlob
+  // which provides the memory (see BufferBlob::create() in codeBlob.cpp).
+  used += CodeBlob::align_code_offset(cb->header_size());
+
+  // Get heap for given CodeBlob and deallocate its unused tail
+  assert(_heap != NULL, "CodeHeap should not be NULL");
+  _heap->deallocate_tail(cb, used);
+  // Adjust the sizes of the CodeBlob
+  cb->adjust_size(used);
+}
+
+
  void CodeCache::commit(CodeBlob* cb) {
    // this is called by nmethod::nmethod, which must already own 
CodeCache_lock
    assert_locked_or_safepoint(CodeCache_lock);
diff -r 27d48c01c0cf src/share/vm/code/codeCache.hpp
--- a/src/share/vm/code/codeCache.hpp    Fri Sep 14 16:39:43 2018 +0800
+++ b/src/share/vm/code/codeCache.hpp    Mon Sep 17 10:57:24 2018 +0800
@@ -78,6 +78,7 @@
    static int alignment_unit();                      // guaranteed 
alignment of all CodeBlobs
    static int alignment_offset();                    // guaranteed 
offset of first CodeBlob byte within alignment unit (i.e., allocation 
header)
    static void free(CodeBlob* cb);                   // frees a CodeBlob
+  static void free_unused_tail(CodeBlob* cb, size_t used); // frees the 
unused tail of a CodeBlob (only used by TemplateInterpreter::initialize())
    static void flush();                              // flushes all 
CodeBlobs
    static bool contains(void *p);                    // returns whether 
p is included
    static void blobs_do(void f(CodeBlob* cb));       // iterates over 
all CodeBlobs
diff -r 27d48c01c0cf src/share/vm/code/stubs.cpp
--- a/src/share/vm/code/stubs.cpp    Fri Sep 14 16:39:43 2018 +0800
+++ b/src/share/vm/code/stubs.cpp    Mon Sep 17 10:57:24 2018 +0800
@@ -24,6 +24,7 @@

  #include "precompiled.hpp"
  #include "code/codeBlob.hpp"
+#include "code/codeCache.hpp"
  #include "code/stubs.hpp"
  #include "memory/allocation.inline.hpp"
  #include "oops/oop.inline.hpp"
@@ -89,6 +90,15 @@
  }


+void StubQueue::deallocate_unused_tail() {
+  CodeBlob* blob = CodeCache::find_blob((void*)_stub_buffer);
+  CodeCache::free_unused_tail(blob, used_space());
+  // Update the limits to the new, trimmed CodeBlob size
+  _buffer_size = blob->content_size();
+  _buffer_limit = blob->content_size();
+}
+
+
  Stub* StubQueue::stub_containing(address pc) const {
    if (contains(pc)) {
      for (Stub* s = first(); s != NULL; s = next(s)) {
diff -r 27d48c01c0cf src/share/vm/code/stubs.hpp
--- a/src/share/vm/code/stubs.hpp    Fri Sep 14 16:39:43 2018 +0800
+++ b/src/share/vm/code/stubs.hpp    Mon Sep 17 10:57:24 2018 +0800
@@ -216,12 +216,15 @@
    void  remove_first(int n);                     // remove the first n 
stubs in the queue
    void  remove_all();                            // remove all stubs 
in the queue

+  void deallocate_unused_tail();                 // deallocate the 
unused tail of the underlying CodeBlob
+                                                 // only used from 
TemplateInterpreter::initialize()
    // Iteration
    static void queues_do(void f(StubQueue* s));   // call f with each 
StubQueue
    void  stubs_do(void f(Stub* s));               // call f with all stubs
    Stub* first() const                            { return 
number_of_stubs() > 0 ? stub_at(_queue_begin) : NULL; }
    Stub* next(Stub* s) const                      { int i = index_of(s) 
+ stub_size(s);
-                                                   if (i == 
_buffer_limit) i = 0;
+                                                   // Only wrap around 
in the non-contiguous case (see stubss.cpp)
+                                                   if (i == 
_buffer_limit && _queue_end < _buffer_limit) i = 0;
                                                     return (i == 
_queue_end) ? NULL : stub_at(i);
                                                   }

diff -r 27d48c01c0cf src/share/vm/interpreter/templateInterpreter.cpp
--- a/src/share/vm/interpreter/templateInterpreter.cpp    Fri Sep 14 
16:39:43 2018 +0800
+++ b/src/share/vm/interpreter/templateInterpreter.cpp    Mon Sep 17 
10:57:24 2018 +0800
@@ -50,6 +50,8 @@
      _code = new StubQueue(new InterpreterCodeletInterface, code_size, 
NULL,
                            "Interpreter");
      InterpreterGenerator g(_code);
+    // Free the unused memory not occupied by the interpreter and the stubs
+    _code->deallocate_unused_tail();
      if (PrintInterpreter) print();
    }

diff -r 27d48c01c0cf src/share/vm/memory/heap.cpp
--- a/src/share/vm/memory/heap.cpp    Fri Sep 14 16:39:43 2018 +0800
+++ b/src/share/vm/memory/heap.cpp    Mon Sep 17 10:57:24 2018 +0800
@@ -227,6 +227,22 @@
  }


+void CodeHeap::deallocate_tail(void* p, size_t used_size) {
+  assert(p == find_start(p), "illegal deallocation");
+  // Find start of HeapBlock
+  HeapBlock* b = (((HeapBlock *)p) - 1);
+  assert(b->allocated_space() == p, "sanity check");
+  size_t used_number_of_segments = size_to_segments(used_size + 
header_size());
+  size_t actual_number_of_segments = b->length();
+  guarantee(used_number_of_segments <= actual_number_of_segments, "Must 
be!");
+  guarantee(b == block_at(_next_segment - actual_number_of_segments), 
"Intermediate allocation!");
+  size_t number_of_segments_to_deallocate = actual_number_of_segments - 
used_number_of_segments;
+  _next_segment -= number_of_segments_to_deallocate;
+  mark_segmap_as_free(_next_segment, _next_segment + 
number_of_segments_to_deallocate);
+  b->initialize(used_number_of_segments);
+}
+
+
  void CodeHeap::deallocate(void* p) {
    assert(p == find_start(p), "illegal deallocation");
    // Find start of HeapBlock
diff -r 27d48c01c0cf src/share/vm/memory/heap.hpp
--- a/src/share/vm/memory/heap.hpp    Fri Sep 14 16:39:43 2018 +0800
+++ b/src/share/vm/memory/heap.hpp    Mon Sep 17 10:57:24 2018 +0800
@@ -134,6 +134,12 @@
    // Memory allocation
    void* allocate  (size_t size, bool is_critical);  // allocates a 
block of size or returns NULL
    void  deallocate(void* p);                     // deallocates a block
+  // Free the tail of segments allocated by the last call to 
'allocate()' which exceed 'used_size'.
+  // ATTENTION: this is only safe to use if there was no other call to 
'allocate()' after
+  //            'p' was allocated. Only intended for freeing memory 
which would be otherwise
+  //            wasted after the interpreter generation because we 
don't know the interpreter size
+  //            beforehand and we also can't easily relocate the 
interpreter to a new location.
+  void  deallocate_tail(void* p, size_t used_size);

    // Attributes
    char* low_boundary() const                     { return 
_memory.low_boundary (); }
diff -r 27d48c01c0cf src/share/vm/runtime/init.cpp
--- a/src/share/vm/runtime/init.cpp    Fri Sep 14 16:39:43 2018 +0800
+++ b/src/share/vm/runtime/init.cpp    Mon Sep 17 10:57:24 2018 +0800
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 1997, 2014, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 1997, 2018, 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
@@ -54,6 +54,14 @@
  void os_init_globals();        // depends on VM_Version_init, before 
universe_init
  void stubRoutines_init1();
  jint universe_init();          // depends on codeCache_init and 
stubRoutines_init
+#if INCLUDE_ALL_GCS
+// depends on universe_init, must be before interpreter_init (currently 
only on SPARC)
+#if defined(SPARC) || defined(MIPS)
+void g1_barrier_stubs_init();
+#else
+void g1_barrier_stubs_init() {};
+#endif
+#endif
  void interpreter_init();       // before any methods loaded
  void invocationCounter_init(); // before any methods loaded
  void marksweep_init();
@@ -106,7 +114,10 @@
    if (status != JNI_OK)
      return status;

-  interpreter_init();  // before any methods loaded
+#if INCLUDE_ALL_GCS
+  g1_barrier_stubs_init();   // depends on universe_init, must be 
before interpreter_init
+#endif
+  interpreter_init();        // before any methods loaded
    invocationCounter_init();  // before any methods loaded
    marksweep_init();
    accessFlags_init();

----- 8< -------- 8< -------- 8< -------- 8< -------- 8< -------- 8< ---


在 2018年09月17日 15:38, David Holmes 写道:
> Hi Leslie,
>
> Patches must be submitted using OpenJDK infrastructure for legal 
> purposes. Please get someone to host your patch on 
> cr.openjdk.java.net, or else (if it is small) inline it in your email.
>
> Thanks,
> David
>
> On 17/09/2018 5:26 PM, Leslie Zhai wrote:
>> Hi all,
>>
>> I would like to backport the fix for:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8166317
>>
>> To OpenJDK 8 updates dev:
>>
>> http://hg.openjdk.java.net/jdk8u/jdk8u-dev
>>
>> The fix is mostly the same as the version that was committed in 10 
>> except that there are:
>>
>> * no _code_end or _data_end private members in the 
>> hotspot/src/share/vm/code/codeBlob.hpp
>>
>> * no get_code_heap(...) function in the 
>> hotspot/src/share/vm/code/codeCache.cpp
>>
>> in OpenJDK 8, so I just:
>>
>> * use _data_offset private member to update code_end() and data_end() 
>> in the adjust_size
>>
>> * use _heap private member to call deallocate_tail(...) function
>>
>> * leave the opportunity for MIPS if need to override 
>> `g1_barrier_stubs_init`
>>
>> Here is the patch:
>>
>> https://raw.githubusercontent.com/xiangzhai/jdk8u-dev/master/JDK-8166317-Backport.patch 
>>
>>
>> Please review it and give me some comments, thanks a lot!
>>
>> ----- 8< -------- 8< -------- 8< -------- 8< -------- 8< -------- 8< ---
>>
>> $ jtreg -vmoptions:"-XX:+UnlockDiagnosticVMOptions 
>> -XX:+PrintInterpreter" 
>> -dir:/home/loongson/zhaixiang/jdk8-mips/jdk/test -verbose:all 
>> -exclude:/home/loongson/zhaixiang/jdk8-mips/jdk/test/ProblemList.txt 
>> -conc:2 -Xmx512m -a -ignore:quiet -timeoutFactor:5 -agentvm 
>> -testjdk:/home/loongson/zhaixiang/jdk8-mips/build/linux-mips64-normal-server-release/images/j2sdk-image 
>> com/sun/jdi/AccessSpecifierTest.java
>>
>> * Applied the patch, theres no more waste in the CodeCache after 
>> interpreter generation and the output of -XX:+PrintInterpreter looks 
>> as follows for jdk8u-dev on mips64el:
>>
>> Agent[0].stdout: 
>> ----------------------------------------------------------------------
>> Agent[0].stdout: Interpreter
>> Agent[0].stdout:
>> Agent[0].stdout: code size        =    163K bytes
>> Agent[0].stdout: total space      =    163K bytes
>> Agent[0].stdout: wasted space     =      0K bytes
>> Agent[0].stdout:
>> Agent[0].stdout: # of codelets    =    263
>> Agent[0].stdout: avg codelet size =    636 bytes
>>
>> ...
>>
>> AccessSpecifierTest: passed
>>
>> * Dropped the patch, the used/wasted ratio for the interpreter part 
>> of the code cache (which can be dumped with -XX:+PrintInterpreter) 
>> looks as follows for jdk8u-dev on mips64el:
>>
>> Agent[0].stdout: 
>> ----------------------------------------------------------------------
>> Agent[0].stdout: Interpreter
>> Agent[0].stdout:
>> Agent[0].stdout: code size        =    163K bytes
>> Agent[0].stdout: total space      =    499K bytes
>> Agent[0].stdout: wasted space     =    336K bytes
>> Agent[0].stdout:
>> Agent[0].stdout: # of codelets    =    263
>> Agent[0].stdout: avg codelet size =    636 bytes
>>
>> ...
>>
>> STDOUT:
>> #
>> # A fatal error has been detected by the Java Runtime Environment:
>> #
>> #  Internal Error (interpreter.hpp:109), pid=23725, 
>> tid=0x000000fff2f9b1f0
>> #  guarantee(codelet_size > 0 && (size_t)codelet_size > 2*K) failed: 
>> not enough space for interpreter generation
>>
>>
>> Thanks,
>>
>> Leslie Zhai
>>
>>




More information about the jdk8u-dev mailing list