lgtm, with comments. https://codereview.appspot.com/6345097/diff/1/src/core/SkPictureFlat.h File src/core/SkPictureFlat.h (right): https://codereview.appspot.com/6345097/diff/1/src/core/SkPictureFlat.h#newcode299 src/core/SkPictureFlat.h:299: // Why do we have this ...
12 years, 2 months ago
(2012-07-12 13:43:24 UTC)
#2
On 2012/07/12 13:43:24, junov1 wrote: > lgtm, with comments. > > https://codereview.appspot.com/6345097/diff/1/src/core/SkPictureFlat.h > File src/core/SkPictureFlat.h ...
12 years, 2 months ago
(2012-07-12 14:49:07 UTC)
#3
On 2012/07/12 13:43:24, junov1 wrote:
> lgtm, with comments.
>
> https://codereview.appspot.com/6345097/diff/1/src/core/SkPictureFlat.h
> File src/core/SkPictureFlat.h (right):
>
>
https://codereview.appspot.com/6345097/diff/1/src/core/SkPictureFlat.h#newcod...
> src/core/SkPictureFlat.h:299: // Why do we have this check???
> Good question. To be consistent with style guide, if we remove the check, I
> think element should be by ref instead of ptr?
>
>
https://codereview.appspot.com/6345097/diff/1/src/core/SkPictureFlat.h#newcod...
> src/core/SkPictureFlat.h:361: HASH_BITS = 8,
> Why 8? IWBN to back this up with experimental data.
> Perhaps log a bug to remind us to investigate this.
Very good comment: I just made it up. I will try different values on our current
benchmarks, to see if smaller is good enough, or if larger helps. Regardless, I
will add a comment at least that this value is tweakable, and subject to current
needs.
Issue 6345097: check a hashtable before using a bsearch
(Closed)
Created 12 years, 2 months ago by reed1
Modified 12 years, 2 months ago
Reviewers: junov, Leon, junov1
Base URL: http://skia.googlecode.com/svn/trunk/
Comments: 2