8321137: Reconsider ICStub alignment

Reviewed-by: dlong, eosterlund, mdoerr, fyang, aph

Author: shipilev

src/hotspot/share/code/compiledIC.cpp

@@ -187,7 +187,7 @@ address CompiledIC::stub_address() const {
 // Clears the IC stub if the compiled IC is in transition state
 void CompiledIC::clear_ic_stub() {
   if (is_in_transition_state()) {
-    ICStub* stub = ICStub_from_destination_address(stub_address());
+    ICStub* stub = ICStub::from_destination_address(stub_address());
     stub->clear();
   }
 }

src/hotspot/share/code/icBuffer.cpp

@@ -88,7 +88,7 @@ void ICStub::finalize() {
     CompiledIC *ic = CompiledIC_at(CodeCache::find_compiled(ic_site()), ic_site());
     assert(CodeCache::find_compiled(ic->instruction_address()) != nullptr, "inline cache in non-compiled?");
 
-    assert(this == ICStub_from_destination_address(ic->stub_address()), "wrong owner of ic buffer");
+    assert(this == ICStub::from_destination_address(ic->stub_address()), "wrong owner of ic buffer");
     ic->set_ic_destination_and_value(destination(), cached_value());
   }
 }
@@ -146,11 +146,6 @@ void InlineCacheBuffer::initialize() {
 }
 
 
-ICStub* InlineCacheBuffer::new_ic_stub() {
-  return (ICStub*)buffer()->request_committed(ic_stub_code_size());
-}
-
-
 void InlineCacheBuffer::refill_ic_stubs() {
 #ifdef ASSERT
   ICRefillVerifier* verifier = current_ic_refill_verifier();
@@ -210,7 +205,7 @@ bool InlineCacheBuffer::create_transition_stub(CompiledIC *ic, void* cached_valu
   }
 
   // allocate and initialize new "out-of-line" inline-cache
-  ICStub* ic_stub = new_ic_stub();
+  ICStub* ic_stub = (ICStub*) buffer()->request_committed(ic_stub_code_size());
   if (ic_stub == nullptr) {
 #ifdef ASSERT
     ICRefillVerifier* verifier = current_ic_refill_verifier();
@@ -219,9 +214,18 @@ bool InlineCacheBuffer::create_transition_stub(CompiledIC *ic, void* cached_valu
     return false;
   }
 
+#ifdef ASSERT
+  {
+    ICStub* rev_stub = ICStub::from_destination_address(ic_stub->code_begin());
+    assert(ic_stub == rev_stub,
+           "ICStub mapping is reversible: stub=" PTR_FORMAT ", code=" PTR_FORMAT ", rev_stub=" PTR_FORMAT,
+           p2i(ic_stub), p2i(ic_stub->code_begin()), p2i(rev_stub));
+  }
+#endif
+
   // If an transition stub is already associate with the inline cache, then we remove the association.
   if (ic->is_in_transition_state()) {
-    ICStub* old_stub = ICStub_from_destination_address(ic->stub_address());
+    ICStub* old_stub = ICStub::from_destination_address(ic->stub_address());
     old_stub->clear();
   }
 
@@ -234,13 +238,13 @@ bool InlineCacheBuffer::create_transition_stub(CompiledIC *ic, void* cached_valu
 
 
 address InlineCacheBuffer::ic_destination_for(CompiledIC *ic) {
-  ICStub* stub = ICStub_from_destination_address(ic->stub_address());
+  ICStub* stub = ICStub::from_destination_address(ic->stub_address());
   return stub->destination();
 }
 
 
 void* InlineCacheBuffer::cached_value_for(CompiledIC *ic) {
-  ICStub* stub = ICStub_from_destination_address(ic->stub_address());
+  ICStub* stub = ICStub::from_destination_address(ic->stub_address());
   return stub->cached_value();
 }
 

src/hotspot/share/code/icBuffer.hpp

@@ -62,16 +62,27 @@ class ICStub: public Stub {
   // General info
   int     size() const                           { return _size; }
 
-  // ICStub_from_destination_address looks up Stub* address from code entry address,
-  // which unfortunately means the stub head should be at the same alignment as the code.
-  static  int alignment()                        { return CodeEntryAlignment; }
+  // To be cautious, we want to make sure that each ICStub is in a separate instruction
+  // cache line. This would allow for piggybacking on instruction cache coherency on
+  // some architectures to order the updates to ICStub and setting the destination to
+  // the ICStub. Note that cache line size might be larger than CodeEntryAlignment
+  // that is normal alignment for CodeBlobs.
+  static int alignment()                         { return DEFAULT_CACHE_LINE_SIZE; }
+
+  // Aligning the code section is normally done for performance reasons, which is not
+  // required for ICStubs, as these stubs are transitional. Setting code alignment
+  // to CodeEntryAlignment would waste a lot of memory in ICBuffer. Aligning to
+  // word size should be enough. This also offsets the costs of aligning the entire
+  // ICStub to cache line (see above), as smaller code alignment would allow ICStub
+  // to fit a _single_ cache line.
+  static int code_alignment()                    { return HeapWordSize; }
 
  public:
   // Creation
   void set_stub(CompiledIC *ic, void* cached_value, address dest_addr);
 
   // Code info
-  address code_begin() const                     { return align_up((address)this + sizeof(ICStub), CodeEntryAlignment); }
+  address code_begin() const                     { return align_up((address)this + sizeof(ICStub), code_alignment()); }
   address code_end() const                       { return (address)this + size(); }
 
   // Call site info
@@ -88,18 +99,15 @@ class ICStub: public Stub {
   void    print()             PRODUCT_RETURN;
 
   // Creation
-  friend ICStub* ICStub_from_destination_address(address destination_address);
+  static inline ICStub* from_destination_address(address destination_address) {
+    ICStub* stub = (ICStub*) align_down(destination_address - sizeof(ICStub), alignment());
+#ifdef ASSERT
+    stub->verify();
+#endif
+    return stub;
+  }
 };
 
-// ICStub Creation
-inline ICStub* ICStub_from_destination_address(address destination_address) {
-  ICStub* stub = (ICStub*) (destination_address - align_up(sizeof(ICStub), CodeEntryAlignment));
-  #ifdef ASSERT
-  stub->verify();
-  #endif
-  return stub;
-}
-
 #ifdef ASSERT
 // The ICRefillVerifier class is a stack allocated RAII object used to
 // detect if a failed IC transition that required IC stub refilling has
@@ -151,8 +159,6 @@ class InlineCacheBuffer: public AllStatic {
 
   static StubQueue* buffer()                         { return _buffer;         }
 
-  static ICStub* new_ic_stub();
-
   // Machine-dependent implementation of ICBuffer
   static void    assemble_ic_buffer_code(address code_begin, void* cached_value, address entry_point);
   static address ic_buffer_entry_point  (address code_begin);

src/hotspot/share/code/stubs.cpp

@@ -73,9 +73,15 @@ StubQueue::StubQueue(StubInterface* stub_interface, int buffer_size,
     vm_exit_out_of_memory(size, OOM_MALLOC_ERROR, "CodeCache: no room for %s", name);
   }
   _stub_interface  = stub_interface;
-  _buffer_size     = blob->content_size();
-  _buffer_limit    = blob->content_size();
-  _stub_buffer     = blob->content_begin();
+
+  // The code blob alignment can be smaller than the requested stub alignment.
+  // Make sure we put the stubs at their requested alignment by aligning the buffer base and limits.
+  address aligned_start = align_up(blob->content_begin(), stub_alignment());
+  address aligned_end = align_down(blob->content_end(), stub_alignment());
+  int aligned_size = aligned_end - aligned_start;
+  _buffer_size     = aligned_size;
+  _buffer_limit    = aligned_size;
+  _stub_buffer     = aligned_start;
   _queue_begin     = 0;
   _queue_end       = 0;
   _number_of_stubs = 0;
@@ -94,8 +100,11 @@ 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();
+  address aligned_start = align_up(blob->content_begin(), stub_alignment());
+  address aligned_end = align_down(blob->content_end(), stub_alignment());
+  int aligned_size = aligned_end - aligned_start;
+  _buffer_size = aligned_size;
+  _buffer_limit = aligned_size;
 }
 
 Stub* StubQueue::stub_containing(address pc) const {

src/hotspot/share/code/stubs.hpp

@@ -49,7 +49,7 @@ class Mutex;
 //               |        |    |
 //               |  data  |    |
 //               |        |    |
-// code_begin -->|--------|    |       <--- aligned by CodeEntryAlignment
+// code_begin -->|--------|    |       <--- aligned by code_alignment()
 //               |        |    |
 //               |        |    |
 //               |  code  |    | size
@@ -99,6 +99,7 @@ class StubInterface: public CHeapObj<mtCode> {
   // General info/converters
   virtual int     size(Stub* self) const                   = 0; // the total size of the stub in bytes (must be a multiple of HeapWordSize)
   virtual int     alignment() const                        = 0; // computes the alignment
+  virtual int     code_alignment() const                   = 0; // computes the code alignment
 
   // Code info
   virtual address code_begin(Stub* self) const             = 0; // points to the first code byte
@@ -127,6 +128,7 @@ class StubInterface: public CHeapObj<mtCode> {
     /* General info */                                     \
     virtual int     size(Stub* self) const                 { return cast(self)->size(); }          \
     virtual int     alignment() const                      { return stub::alignment(); }           \
+    virtual int     code_alignment() const                 { return stub::code_alignment(); }      \
                                                            \
     /* Code info */                                        \
     virtual address code_begin(Stub* self) const           { return cast(self)->code_begin(); }    \
@@ -154,9 +156,10 @@ class StubQueue: public CHeapObj<mtCode> {
   Mutex* const   _mutex;                         // the lock used for a (request, commit) transaction
 
   void  check_index(int i) const                 { assert(0 <= i && i < _buffer_limit && i % stub_alignment() == 0, "illegal index"); }
+  void  check_stub_align(Stub* s) const          { assert(((intptr_t)s) % stub_alignment() == 0, "incorrect stub alignment"); }
   bool  is_contiguous() const                    { return _queue_begin <= _queue_end; }
   int   index_of(Stub* s) const                  { int i = (int)((address)s - _stub_buffer); check_index(i); return i; }
-  Stub* stub_at(int i) const                     { check_index(i); return (Stub*)(_stub_buffer + i); }
+  Stub* stub_at(int i) const                     { check_index(i); Stub* s = (Stub*)(_stub_buffer + i); check_stub_align(s); return s; }
   Stub* current_stub() const                     { return stub_at(_queue_end); }
 
   // Stub functionality accessed via interface

src/hotspot/share/interpreter/interpreter.hpp

@@ -60,9 +60,10 @@ class InterpreterCodelet: public Stub {
   // General info/converters
   int     size() const                           { return _size; }
   static  int alignment()                        { return HeapWordSize; }
+  static  int code_alignment()                   { return CodeEntryAlignment; }
 
   // Code info
-  address code_begin() const                     { return align_up((address)this + sizeof(InterpreterCodelet), CodeEntryAlignment); }
+  address code_begin() const                     { return align_up((address)this + sizeof(InterpreterCodelet), code_alignment()); }
   address code_end() const                       { return (address)this + size(); }
 
   // Debugging