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