[9] RFR(M): 8054292 : code comments leak in fastdebug builds

[Vladimir Kozlov]

On 8/29/14 12:16 PM, David Chase wrote:
>
> On 2014-08-28, at 1:51 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>> On 8/28/14 6:28 AM, David Chase wrote:
>>>
>>> Revised changes: http://cr.openjdk.java.net/~drchase/8054292/webrev-for-leak-checks.12/
>
>>> Added placement-new-constructor call in InterpreterCodelet (to allow that extra assert).
>>
>> This one I am not sure. It could hide potential memory leak when constructor overwrite previously stored _strings values. But I agree that it is the simplest change to enable the assert. And based on what I see in current code it should not cause a leak because we assign default NULL to _strings several times until the final store in ~CodeletMark().
>>
>> In short, I accept current change but, please, file RFE (starter task) to clean this code. As I said before strings recording for InterpreterCodelet could be done differently (do not do it generally for Stub class).
>
> Chris seemed to think that though this could leak, it was bounded because we only generate the interpreter once.
> But I’ll file that bug once this one is pushed.

I mostly want to clean that code for easy understanding and already then 
for leak avoidance.

>
>>> Tested fastdebug and not-debug (calls itself “release”) builds w/ jtreg, fastdebug with PrintAsm enabled.
>>> Did 64-hour leak check with KitchenSink; any leaks seem to be below the what’s-currently-compiled noise threshold.
>>>
>>> Sanity check: is “optimized build” the default if I just run configure?
>>
>> No, the default is 'product' Hotspot build. I would suggest to build 'optimized' VM separately (it looks like 'optimized' target for whole forest does not work) and test it for leaks.
>
> Done, and there appeared to be no leaks in CTW (not in “code”, which is where they showed up before).
>
> Good to go?

Yes, thumbs up from me.

Thanks,
Vladimir

>
> David
>