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

[Joseph Provino]

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
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>
>>>>
>>>
>>
>