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

[Thomas Stüfe]

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> 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] On
> > Behalf Of Thomas Stüfe
> > Sent: Freitag, 20. November 2015 11:49
> > To: 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20151125/0dc9c964/attachment.html>