It would be nice that the implementation inside MemoryPressureHander::respondToMemoryPressure are shareable between mac and iOS.
Created attachment 121913 [details] Move memory pressure handling code inside a new function (releaseMemory) so that we could shared it between mac and iOS.
<rdar://problem/10670958>
Comment on attachment 121913 [details] Move memory pressure handling code inside a new function (releaseMemory) so that we could shared it between mac and iOS. View in context: https://bugs.webkit.org/attachment.cgi?id=121913&action=review > Source/WebCore/platform/mac/MemoryPressureHandlerMac.mm:118 > + releaseMemory(false); This boolean should ideally be an enum: releaseMemory(Normal) releaseMemory(Critical) > Source/WebCore/platform/mac/MemoryPressureHandlerMac.mm:123 > +void MemoryPressureHandler::releaseMemory(bool critical) > +{ What do you think about moving this to a separate common file? As it is, 90% of this file is blacklisted for iOS. Separate files are easier to maintain that #ifdef imho (although the file would be very simple in this case). If you choose the #ifdef, do not forget to add || PLATFORM(IOS) to MemoryPressureHandler.cpp. > Source/WebCore/platform/mac/MemoryPressureHandlerMac.mm:125 > - pageCache()->setCapacity(pageCache()->pageCount()/2); > + pageCache()->setCapacity(critical ? 0 : pageCache()->pageCount()/2); Spaces around operator. > Source/WebCore/platform/mac/MemoryPressureHandlerMac.mm:131 > + [nsurlCache setMemoryCapacity:critical ? 0 : [nsurlCache currentMemoryUsage]/2]; Spaces around operator.
(In reply to comment #3) > (From update of attachment 121913 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=121913&action=review > > > Source/WebCore/platform/mac/MemoryPressureHandlerMac.mm:118 > > + releaseMemory(false); > > This boolean should ideally be an enum: > releaseMemory(Normal) > releaseMemory(Critical) > Agree. Currently we only have two levels, we can consider changing this to enum when new levels are introduced. > > Source/WebCore/platform/mac/MemoryPressureHandlerMac.mm:123 > > +void MemoryPressureHandler::releaseMemory(bool critical) > > +{ > > What do you think about moving this to a separate common file? As it is, 90% of this file is blacklisted for iOS. Separate files are easier to maintain that #ifdef imho (although the file would be very simple in this case). > I thought about that, but didn't like the idea of having a separate file with just this one function. > If you choose the #ifdef, do not forget to add || PLATFORM(IOS) to MemoryPressureHandler.cpp. Ok, looks like we already had that for iOS. > > > Source/WebCore/platform/mac/MemoryPressureHandlerMac.mm:125 > > - pageCache()->setCapacity(pageCache()->pageCount()/2); > > + pageCache()->setCapacity(critical ? 0 : pageCache()->pageCount()/2); > > Spaces around operator. > > > Source/WebCore/platform/mac/MemoryPressureHandlerMac.mm:131 > > + [nsurlCache setMemoryCapacity:critical ? 0 : [nsurlCache currentMemoryUsage]/2]; > > Spaces around operator.
Created attachment 122275 [details] Address review comments on previous patch. Address Benjamin's review comments, adding || PLATFORM(IOS) to MemoryPressureHander.cpp.
Comment on attachment 122275 [details] Address review comments on previous patch. View in context: https://bugs.webkit.org/attachment.cgi?id=122275&action=review > Source/WebCore/platform/MemoryPressureHandler.h:48 > + void releaseMemory(bool critical); I still feel the enum is better, even for only two values here.
Comment on attachment 122275 [details] Address review comments on previous patch. Clearing flags on attachment: 122275 Committed r104873: <http://trac.webkit.org/changeset/104873>
All reviewed patches have been landed. Closing bug.