Summary: | [Qt][WK2] MappedMemoryPool is unbounded | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Balazs Kelemen <kbalazs> | ||||||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED WONTFIX | ||||||||||
Severity: | Normal | CC: | kimmo.t.kinnunen, laszlo.gombos | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Balazs Kelemen
2010-12-20 08:09:03 PST
Created attachment 77009 [details]
Patch
Created attachment 77042 [details]
Patch
Eliminated the pointless base class, otherwise the same as the last one.
Comment on attachment 77042 [details]
Patch
First need to solve #51474 to avoid conflicts.
Created attachment 77989 [details]
Patch
This addresses a quite serious problem with MemoryMappedPool, good. However I'm not really convinced that anybody should spend that much time polishing current MemoryMappedPool before it really is working well. To me, this patch just adds complexity on top of simple implementation. To me, address space running out is the least of the worries for MemoryMappedPool. The simple implementation was initially done just to get WK2 on Qt working -- it is not meant as end-all design. I opened another bug that discusses current problems: https://bugs.webkit.org/show_bug.cgi?id=51984 Some of the problems are amplified by this patch. At least: - this patch adds even more calls that don't check errors (file opens, mmaps) - this patch adds polish (enums instead of bools) to the isFree flag, which is incorrect anyway Comment on attachment 77989 [details]
Patch
We should make a consensus about do
we really need the pooling
optimization. Postpone this work until
it has been done.
(In reply to comment #6) > (From update of attachment 77989 [details]) > We should make a consensus about do > we really need the pooling > optimization. Postpone this work until > it has been done. Or, I can add that pooling if it makes you happy. Should I do that and do another patch? (In reply to comment #7) > (In reply to comment #6) > > (From update of attachment 77989 [details] [details]) > > We should make a consensus about do > > we really need the pooling > > optimization. Postpone this work until > > it has been done. > > Or, I can add that pooling if it makes you happy. Should I do that and do another patch? It's not about my happiness. If your experiences (benchmarks, profiling, user experience on device, etc.) does not justify the value of the optimization then it should be removed. We can reintroduce it in the future if mmap will be a bottleneck. |