[webkit-reviews] review canceled: [Bug 196160] [JSC] Butterfly allocation from LargeAllocation should try "realloc" behavior if collector thread is not active : [Attachment 366054] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has canceled Yusuke Suzuki
<ysuzuki at apple.com>'s request for review:
Bug 196160: [JSC] Butterfly allocation from LargeAllocation should try
"realloc" behavior if collector thread is not active
https://bugs.webkit.org/show_bug.cgi?id=196160

Attachment 366054: Patch

https://bugs.webkit.org/attachment.cgi?id=366054&action=review




--- Comment #43 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 366054
  --> https://bugs.webkit.org/attachment.cgi?id=366054
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366054&action=review

>>> Source/JavaScriptCore/heap/CompleteSubspace.cpp:189
>>> +	     m_largeAllocations.append(oldAllocation);
>> 
>> Why do we need to do this if we failed to allocate?
> 
> Because we removed this from Subspace in LargeAllocation::tryReallocate. If
we do not remove oldAllocation from the linked list in "tryReallocate", we miss
the chance to remove oldAllocation from Subspace's doubly linked list when
"realloc" succeeds since address may be changed by "realloc".

I changed this code a bit: moving 

    if (isOnList())
	remove();

operations out of tryReallocate to make code cleaner.

>>> Source/JavaScriptCore/heap/LargeAllocation.cpp:77
>>> +	 // This includes padding at the end of the allocation to maintain the
distancing constraint.
>> 
>> We should remove this.
> 
> I'll remove this in "distancing removing patch" side :)

Removed. So I'll update the patch.