[Webkit-unassigned] [Bug 94810] Vector::shrinkToFit should use realloc when canCopyWithMemcpy is true

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=94810





--- Comment #9 from Benjamin Poulain <benjamin at webkit.org>  2012-08-28 08:37:59 PST ---
> Isn't this the worst case of using realloc (causing fragments)? If the memory allocated isn't small, and realloc() would keep the address unchanged, it saves a memcpy on a large block. If realloc() uses different addresses, it is same as malloc/memcpy/free.
> 
> I understand this depends very much on the actual malloc algorithm. But what Vector is trying to do here is just a re-alloc. It is very natural to use realloc, and lets the malloc algorithm to decide what to do for best performance.

I am not sure what you arguing for? (against?)
What you say is correct, and I agree using realloc seems like the good thing to do.

But you should not make changes like this one without measuring the benefits. You are modifying a abstract data type used everywhere in WebKit in a non obvious way, you should be able to prove the change is correct.

It is not obvious if
1) This will increase memory fragmentation because realloc is not flexible for shrink.
2) This will not provide any improvement and it is just extra code to support.
3) Fastmalloc does the right thing and the patch avoid some memcopy.

(3) should go in WebKit, (1) and (2) should just be discarded. I only ask you make sure you have the third case.

> Probably adding an ENABLE switch is better?

No, please. ENABLE switch is a bad idea here.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.