RFR/advice: 8054292 : Code comments leak in fastdebug builds
vladimir.kozlov at oracle.com
Thu Aug 14 18:18:46 UTC 2014
On 8/14/14 10:51 AM, David Chase wrote:
> Correction — we do NOT copy strings out, we instead copy the pointer,
> hence the need to zero the strings in the CodeBuffer after a copy-out
> if we reclaim non-zeroes strings in the destructor.
>> You added _code.free_strings() in C1 but you missed the same case in C2 when compilation is bailed out and strings are not copied to nmethod.
>> In scratch_emit_size() you should free strings instead of setting NULL since they are not copied at that place.
> I think I will modify the code to do the reclamation in the destructor, because otherwise
> I’ll lose track of all the places where the strings go uncopied (and if I don’t, the next guy
> will). There’s many more failure paths than success paths, so it is easier to track success
> and zero the pointer after copying, and I can be pretty sure that I have executed the
> success paths (but not the failure paths).
Yes, if you always zero after a copy (it is only one place free_strings(), right?).
And do not zero in other places (to not create dangling pointers) this should work.
> Or are you saying that safest is simply to deep-copy always and always free strings
> in the destructor and never set them NULL?
I meant it would be simplest and clean solution from language point of view but you will pay performance and memory for
that. So I prefer your suggestion above.
>> CodeBuffer::free_strings() should set _Strings to NULL after freeing them.
> A side-effect of free_strings is that _Strings.isNull becomes true.
> I’ll comment this.
>> Indention is wrong in new code - should be 2 spaces.
> Will fix.
>> And no spaces around arrow: "cb -> “.
> I’ll come around with another webrev after another round of testing.
More information about the hotspot-compiler-dev