review request for JDK-8013461 There is a symbol AsyncGetCallTrace in libjvm.symbols that does not exist in minimal/libjvm.a when DEBUG_LEVEL == release

[serguei.spitsyn at oracle.com]

The fix is good.

Thanks,
Serguei


On 5/22/13 12:27 PM, Joseph Provino wrote:
> Is there a consensus what is in the webrev is okay?
>
> The change is to include forte.cpp in the minimal jvm but to
> conditionalize the code so that only AsyncGetCallTrace()
> is defined with the minimal jvm.
>
> Webrev is here: http://cr.openjdk.java.net/~jprovino/8013461/webrev.01
>
>     JDK-8013461 https://jbs.oracle.com/bugs/browse/JDK-8013461
>     There is a symbol AsyncGetCallTrace in libjvm.symbols that does 
> not exist in
>     minimal/libjvm.a when DEBUG_LEVEL == release
>
> joe
>
> On 05/21/2013 04:52 PM, JOSEPH PROVINO wrote:
>>
>> On 5/21/2013 4:00 PM, Oleg Mazurov wrote:
>>> Though formally not part of the Solaris Studio team any more here is 
>>> my opinion based on my recollection of how I implemented interaction 
>>> with the JVM via AsyncGetCallTrace.
>>> It's looked up using dlsym. If the symbol is not there Java 
>>> callstack collection is shut down. I understand in your case even 
>>> JVMTI is not there so the dlsym call will not be made.
>>> From that perspective there is no difference whether the symbol is 
>>> present and returns an error code or not present at all.
>>
>> Oleg, then it sounds like what we have will work.
>>
>> Thanks for the quick reply.
>>
>> joe
>>
>>>
>>>     -- Oleg
>>>
>>> On 5/21/2013 12:19 PM, JOSEPH PROVINO wrote:
>>>>
>>>> On 5/21/2013 3:16 PM, serguei.spitsyn at oracle.com wrote:
>>>>> On 5/21/13 11:26 AM, JOSEPH PROVINO wrote:
>>>>>>
>>>>>> On 5/21/2013 2:23 PM, Staffan Larsen wrote:
>>>>>>> On 21 maj 2013, at 17:35, JOSEPH PROVINO 
>>>>>>> <joseph.provino at oracle.com> wrote:
>>>>>>>
>>>>>>>> On 5/21/2013 3:06 AM, David Holmes wrote:
>>>>>>>>> Hi Staffan,
>>>>>>>>>
>>>>>>>>> On 21/05/2013 4:49 PM, Staffan Larsen wrote:
>>>>>>>>>> On 21 maj 2013, at 04:34, David Holmes 
>>>>>>>>>> <David.Holmes at oracle.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> <added servicability>
>>>>>>>>>>>
>>>>>>>>>>> Hi Joe,
>>>>>>>>>>>
>>>>>>>>>>> As I have previously stated you copied the struct 
>>>>>>>>>>> definitions instead of moving them outside the ifdef.
>>>>>>>>>>>
>>>>>>>>>>> Serviceability folk: we are particularly interested in 
>>>>>>>>>>> whether the use of ticks_no_class_load is deemed appropriate 
>>>>>>>>>>> in this situation. Who will be consuming this value?
>>>>>>>>>> Since you have opted for the simple fix of having an exported 
>>>>>>>>>> but non-functional AsyncGetCallTrace instead of actually 
>>>>>>>>>> removing the symbol from the symbol files (which is the 
>>>>>>>>>> proposed solution in the bug report),
>>>>>>>>> That would be a simpler solution semantically but the only way 
>>>>>>>>> I can see to do that is to use a text replacement mechanism in 
>>>>>>>>> the build files - as is done for the dynamic vtable symbols. I 
>>>>>>>>> find that less appealing than simply exporting an interface 
>>>>>>>>> that is configured to report an error (which is essentially 
>>>>>>>>> what all the optional interfaces do under the minimal VM).
>>>>>>>>>
>>>>>>>>>> I would like you to include a comment about this in the 
>>>>>>>>>> source. Right now it's very unclear why there is an exported 
>>>>>>>>>> function that only returns an error.
>>>>>>>>>>
>>>>>>>>>> As to the appropriate return value, I don't know. The only 
>>>>>>>>>> caller should be the Sun Studio profiler,
>>>>>> Does anyone know where to find instructions on how to run the 
>>>>>> collector which would get the error return value?
>>>>>>>>>>   and I'm not sure how it will handle this case if ever run. 
>>>>>>>>>> The possible return values aren't very well documented.
>>>>>>>>> I guess we need to try and run it to find out.
>>>>>>>> Okay, do either of you feel strongly about how this should be 
>>>>>>>> fixed -- return an error or remove the symbol?
>>>>>>> No, I don't feel strongly either way, but a comment in the code 
>>>>>>> would be nice.
>>>>>> How much effort should I put into finding out what Sun Studio 
>>>>>> profiler does when it gets -1?
>>>>>
>>>>> Let's ask the Solaris Studio guys directly.
>>>>> I'm adding Oleg to the mailing list.
>>>>>
>>>>> Oleg,
>>>>>
>>>>> Could you, please, share your view on this problem?
>>>>
>>>> In particular what will the Sun Studio Profiler collector do if it 
>>>> gets the error
>>>>
>>>> trace->num_frames = ticks_no_class_load; // -1
>>>>
>>>> Thanks.
>>>>
>>>> joe
>>>>
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> joe
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> /Staffan
>>>>>>>
>>>>>>>
>>>>>>>> joe
>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>>> /Staffan
>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> David
>>>>>>>>>>>
>>>>>>>>>>> On 21/05/2013 5:10 AM, JOSEPH PROVINO wrote:
>>>>>>>>>>>> The change is to include forte.cpp in the minimal jvm but to
>>>>>>>>>>>> conditionalize the code so that
>>>>>>>>>>>> only AsyncGetCallTrace() is defined with the minimal jvm.
>>>>>>>>>>>>
>>>>>>>>>>>> Webrev is here: 
>>>>>>>>>>>> http://cr.openjdk.java.net/~jprovino/8013461/webrev.00/
>>>>>>>>>>>>
>>>>>>>>>>>>   * JDK-8013461 
>>>>>>>>>>>> <https://jbs.oracle.com/bugs/browse/JDK-8013461>There is
>>>>>>>>>>>>     a symbol AsyncGetCallTrace in libjvm.symbols that does 
>>>>>>>>>>>> not exist in
>>>>>>>>>>>>     minimal/libjvm.a when DEBUG_LEVEL == release
>>>>>>>>>>>> <https://jbs.oracle.com/bugs/browse/JDK-8013461>
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>
>>>>>>>>>>>> joe
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>