RFR: JDK-8199682 Clean up building the saproc library

Magnus Ihse Bursie magnus.ihse.bursie at oracle.com
Fri Mar 16 18:12:08 UTC 2018


Hi Sundar,

I almost missed your mail, since you removed both me and build-dev from the cc list...

> 16 mars 2018 kl. 06:14 skrev Sundararajan Athijegannathan <sundararajan.athijegannathan at oracle.com>:
> 
> Renaming sawindbg as saproc sounds odd. For Linux, Solaris/Unix, we either use /proc & libproc, so calling saproc for those makes sense. But Windows? We have a separate debugger class to load platform specific native library. What is the reason for uniform naming?

This is the only library in the JDK that has a different name on different platform. This clashes with the design of the build system, and requires a clunky workaround. For the upcoming changes in the build system, this goes from an annoyance to a blocker.

No other components have their names based on the OS functionality they use, even if they use vastly different APIs on different platforms; rather they are named after the services they provide to the JDK.

My assumption was that ”saproc” meant ”serviceability agent process handling”, and that this was a reasonable name for all platforms. Also, the source code for all platforms reside in the ”libsaproc” directory, which is consistent with the JDK standard for matching source code to native library.

But if you believe this is an inappropriate name, let’s work together to find a name that works for all platforms. This of course will lead to new names for the current libsaproc.* libraries, and the source code directories.

/Magnus

> 
> -Sundar
> 
> On 16/03/18, 12:19 AM, Magnus Ihse Bursie wrote:
>> 
>> 
>> On 2018-03-15 19:39, Erik Joelsson wrote:
>>> Looks good to me.
>>> 
>>> The removed source files, are those some kind of tests?
>> I don't really know; they have been excluded from the build for all time. My guess is that the Bsd* stuff is, like in the case of the sound libraries, bsd-based stuff that arrived with the mac port (but disabled). The test.c is a trivial main() method which looks more like a left-over adhoc testing from the initial developer. Perhaps someone wants to turn it into a proper test, but it seems like it's not much even to start with. (And hopefully we have much better real test coverage of this now.)
>> 
>> /Magnus
>>> 
>>> /Erik
>>> 
>>> 
>>> On 2018-03-15 11:22, Magnus Ihse Bursie wrote:
>>>> The saproc library has historically been built in quite odd ways on almost all platforms. When the old build system was converted, this was not changed.
>>>> 
>>>> However, now the time has come to streamline this and build this library just as any other.
>>>> 
>>>> The most visible change, perhaps, is that the library is now named saproc on all platforms, even Windows. Other changes include:
>>>> * Don't set flags that is already set by the default flags.
>>>> * Don't set flags that do not have anny effect.
>>>> * Don't subst away the WIN32_LEAN_AND_MEAN definition, it's perfectly okay to have it.
>>>> * Don't set CXX linker on solaris -- this was not needed so no reason to do it.
>>>> * Cleaned up some old hooks for closed code that is no longer needed.
>>>> 
>>>> I have verified this using COMPARE_BUILD. This shows only the expected differences:
>>>> * On all platforms: class file changes for WindbgDebuggerLocal.java.
>>>> * On solaris: some minor symbol differences, since the linker now uses C framework functions instead of C++. (And with symbol changes always comes disasm changes.)
>>>> * On linux: a binary difference for libsaproc.so, but no size/symbol/deps/disasm change.
>>>> * On macosx: no changes at all.
>>>> * On windows: sawindbg.dll is renamed to saproc.dll. When I made a manual comparison between the two files, I found no significant differences.
>>>> 
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8199682
>>>> WebRev: http://cr.openjdk.java.net/~ihse/JDK-8199682-clean-up-saproc/webrev.01
>>>> 
>>>> /Magnus
>>>> 
>>> 
>> 



More information about the serviceability-dev mailing list