RFR (S): 8147910: Cache initial active_processor_count

Thomas Schatzl thomas.schatzl at oracle.com
Mon Jul 25 08:13:12 UTC 2016


Hi David,

On Mon, 2016-07-25 at 14:55 +1000, David Holmes wrote:
> Hi Thomas,
> 
> Thanks again for taking this on.

  thank you for the quick reviews.

> 
> On 22/07/2016 9:36 PM, Thomas Schatzl wrote:
> > 
> > Hi all,
> > 
> >   can I have reviews for this tiny change that adds an API to set
> > the
> > initial active processor count at startup? It also adds that count
> > to
> > the hs_err log file.
> > 
> > It is required to fix JDK-8161993.
> > 
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8147910
> > 
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8147910/webrev/
> src/share/vm/runtime/os.hpp
> 
> Nit: can you change update_initial_active_processor_count() to 
> initialize_initial_active_processor_count() please - otherwise it
> gives 
> the impression that it can be called many times to update the value.
> 
> +     guarantee(_initial_active_processor_count > 0, "Initial active 
> processor count not set yet.")
> 
> An assert will suffice here.
> 

All fixed.

> ---
> 
> src/share/vm/runtime/os.cpp:
> 
> !   // We access the raw value here because using the accessor will
> let 
> its guarantee
> !   // fail if the crash occurs before initialization of this value.
> !   st->print(" (initial active %d)",
> _initial_active_processor_count);
> 
> Good point! Did you find that out the hard way? :)
> 
> I wonder if we should clarify the message in that case:
> 
> (initial active %d - zero if not yet set)

  I think it would be just confusing in the most common case. There is
always the comment in the code that already indicates that a zero value
means not initilialized yet.

New webrevs:
http://cr.openjdk.java.net/~tschatzl/8147910/webrev.0_to_1 (diff) 
http://cr.openjdk.java.net/~tschatzl/8147910/webrev.1 (full)


Thanks,
  Thomas



More information about the hotspot-runtime-dev mailing list