[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: JDK-8199682 Clean up building the saproc library
From:       Sundararajan Athijegannathan <sundararajan.athijegannathan () oracle ! com>
Date:       2018-03-20 15:21:47
Message-ID: 5AB1243B.1020807 () oracle ! com
[Download RAW message or body]

Hi,

Sounds good - so long as we don't have scripts that depend on the old 
name. Or if those could be fixed...

-Sundar

On 20/03/18, 4:54 PM, Magnus Ihse Bursie wrote:
>
> On 2018-03-16 19:12, Magnus Ihse Bursie wrote:
>> 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@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.
> Hi Sundar,
>
> Are you okay with this rationale for changing to saproc, or do you 
> want to discuss this further?
>
> /Magnus
>>
>> /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
>>>>>>
>
[prev in list] [next in list] [prev in thread] [next in thread]