RFR(L): 8143125: [aix] Further Developments for AIX

[Lindenmaier, Goetz]

Hi Thomas,

thanks a lot for doing all these changes.
Looks good now.

Could you add the change comment to the patch before you
Make a webrev next time?  Simplifies sponsoring ;)

Best regards,
  Goetz.

From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
Sent: Mittwoch, 25. November 2015 18:01
To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
Cc: ppc-aix-port-dev at openjdk.java.net; HotSpot Open Source Developers <hotspot-dev at openjdk.java.net>
Subject: Re: RFR(L): 8143125: [aix] Further Developments for AIX

Hi Goetz,

new webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8143125-Further/webrev.01/webrev/

thank you for reviewing! See remarks inline...

On Tue, Nov 24, 2015 at 12:38 PM, Lindenmaier, Goetz <goetz.lindenmaier at sap.com<mailto:goetz.lindenmaier at sap.com>> wrote:
Hi Thomas,

I looked at your change.  It’s good we get these improvements into
openJDK.  But I think we should skip some stuff that's not (yet?)
used there.  I'll sponsor the change. Details:

libperfstat_aix.cpp:

What do you need
static fun_perfstat_partition_total_t g_fun_perfstat_partition_total = NULL;
static fun_perfstat_wpar_total_t    g_fun_perfstat_wpar_total = NULL;
static fun_wpar_getcid_t            g_fun_wpar_getcid = NULL;
for? I think they are never used.

They are now used (for the respective wrapper functions which in turn are used in os_aix.cpp. Sorry for omitting this code.

libperfstat_aix.cpp:161
We support AIX 5.3 with xlC12 only. I think we need not add the older
#defines to openJDK. Also, the comment should be adapted and not mention xlc8 etc.
Also, there are datastructures for 5.2 which can be removed.

Removed the older defines.
I would for now prefer to leave the AIX 5.2 data structures in this file. I am not sure if we could encounter older libperfstat versions on AIX 5.3 too. I plan to rework this coding to get rid of the duplicate structure definitions, but would prefer to do this in a separate patch.


Where is this used?
bool libperfstat::get_wparinfo(wparinfo_t* pwi)
Do we need it if it's not used?

It is now used :) in os_aix.cpp.


libperfstat.hpp:835
I would remove these prototypes commented out.

I removed them.

loadlib_aix.cpp
Why do you do these changes? Please revert them.

I reverted them.

os_aix.hpp:219
What's this good for?
get_brk_at_startup()
get_lowest_allocation_above_brk()

I removed those prototypes.

os_aix.hpp:259
What's this good for?
parse_qsyslib_path()

I removed this prototype.

os_aix.cpp:410
Why do you move query_multipage_support()?

I moved this back to its original position.

os_aix.inline.hpp:164
Why do you reverse the order of the functions here?
The old order is the same as in the related files os_linux.inline.hpp etc.

I reversed the order back to original order.

Best regards,
  Goetz.


Kind Regards, Thomas


> -----Original Message-----
> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net<mailto:hotspot-dev-bounces at openjdk.java.net>] On
> Behalf Of Thomas Stüfe
> Sent: Freitag, 20. November 2015 11:49
> To: ppc-aix-port-dev at openjdk.java.net<mailto:ppc-aix-port-dev at openjdk.java.net>; HotSpot Open Source Developers
> Subject: RFR(L): 8143125: [aix] Further Developments for AIX
>
> Hi all,
>
> please review and sponsor these AIX only changes. Basically, with this
> change we bring the OpenJDK AIX hotspot port up to speed with the
> developments done at SAP in the recent months.
>
> For a more detailled number of changes and fixes, please refer to the bug
> description.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8143125
> webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8143125-
> Further/webrev.00/webrev/index.html
>
> Kind Regards, Thomas