Bug 48484 - Improve memSize calculation in [WebView _setCacheModel]
Summary: Improve memSize calculation in [WebView _setCacheModel]
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-27 16:39 PDT by Pratik Solanki
Modified: 2010-10-27 17:42 PDT (History)
3 users (show)

See Also:


Attachments
Patch (3.72 KB, patch)
2010-10-27 17:16 PDT, Pratik Solanki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pratik Solanki 2010-10-27 16:39:35 PDT
In [WebView _setCacheModel], we use a fudge factor for the memory size


    // As a fudge factor, use 1000 instead of 1024, in case the reported byte 
    // count doesn't align exactly to a megabyte boundary.
    uint64_t memSize = WebMemorySize() / 1024 / 1000;

Instead of doing that, we should just round up the memSize to a multiple of 2 and update the code to check for 4MB/2MB/1MB/512KB memory levels.
Comment 1 Pratik Solanki 2010-10-27 17:16:25 PDT
Created attachment 72118 [details]
Patch
Comment 2 Darin Adler 2010-10-27 17:23:27 PDT
Comment on attachment 72118 [details]
Patch

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

Changes look fine. I have a comment or two, but nothing we couldn’t tweak later.

> WebKit/mac/WebView/WebView.mm:5324
> +    static uint64_t memSize = roundUpToPowerOf2(WebMemorySize() / 1024 / 1024);

Is it really worthwhile to store this in a global? Paying 8 bytes of global memory to avoid doing this math more than once might be the wrong tradeoff since we do this computation only when the cache model is changed.
Comment 3 WebKit Commit Bot 2010-10-27 17:42:45 PDT
Comment on attachment 72118 [details]
Patch

Clearing flags on attachment: 72118

Committed r70732: <http://trac.webkit.org/changeset/70732>
Comment 4 WebKit Commit Bot 2010-10-27 17:42:50 PDT
All reviewed patches have been landed.  Closing bug.