RFR: JDK-8157141 & JDK-8166454: Solaris getisax(2) and meminfo(2) cleanup
Kim Barrett
kim.barrett at oracle.com
Fri Sep 23 01:08:58 UTC 2016
> On Sep 22, 2016, at 6:54 PM, Alan Burlison <Alan.Burlison at oracle.com> wrote:
>
> getisax(2) and meminfo(2) are both Committed interfaces. getisax(2) first appeared in Solaris 10, meminfo(2) first appeared in Solaris 9. Currently they are both accessed via dlsym(3C) which is unnecessary as they will always be present on any version of Solaris that Java9+ will run on. In addition, this patch modifies the use of getisax(2) to probe for the correct buffer size to use so it will continue to work if/when the current 2-word buffer size is exceeded - although of course any newly-returned capability bits will need handling, as is already the case.
>
> I'm submitting these two bugs as a single unit for review as they share common code (os::Solaris::misc_sym_init()) which can be removed if they are fixed together.
>
> getisainfo() manpage:
> http://docs.oracle.com/cd/E53394_01/html/E54765/getisax-2.html
>
> meminfo() manpage:
> https://docs.oracle.com/cd/E53394_01/html/E54765/meminfo-2.html
>
> Bugs: https://bugs.openjdk.java.net/browse/JDK-8157141
> https://bugs.openjdk.java.net/browse/JDK-8166454
> Webrev: http://cr.openjdk.java.net/~alanbur/JDK-8157141%2bJDK-8166454/
>
> JPRT hostpot testset is clean.
>
> --
> Alan Burlison
> --
------------------------------------------------------------------------------
src/os_cpu/solaris_sparc/vm/vm_version_solaris_sparc.cpp
360 uint_t avn = getisax(NULL, 0);
361 uint_t *avs = (uint_t*) alloca(avn * sizeof(uint_t));
362 uint_t av = avs[0];
I see nothing here to fill in the alloca'ed avs array. Shouldn't there
be another call to getisax?
> JPRT hostpot testset is clean.
That suggests the conditionalization on all these features is pretty
good, since it looks to me like that testing didn't use real values.
The use of getisax with an empty array to determine how many elements
to allocate for a real call doesn't obviously follow from the
referenced documentation. "... returns the number of array elements
that contain non-zero values." suggests to me that it should be no
greater than the "n" (second) argument and could be less, as it is the
number of non-zero elements in the first (array) argument that is
returned. Perhaps that's a bug in the documentation?
I think the old code that only supported two elements was fine, except
needing to replace os::solaris::getisax with just getisax. Getting the
actual length and using alloca seems like overkill to me. We're not
looking for any entries beyond the second element. If we ever want to
start doing so, just increase the static length accordingly. This is
assuming the behavior exhibited by the empty array call, e.g. the
result is the number of entries that are available for access, and not
limited by the size of the array argument.
However, the old assertion that the result of getisax <= 2 *is*
inconsistent with the behavior that seems to be expected from using an
empty array to get the available number of entries.
------------------------------------------------------------------------------
More information about the hotspot-runtime-dev
mailing list