[webkit-reviews] review granted: [Bug 207183] [WTF] Try using 75% load factor for HashTable : [Attachment 390168] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 207183: [WTF] Try using 75% load factor for HashTable
https://bugs.webkit.org/show_bug.cgi?id=207183

Attachment 390168: Patch

https://bugs.webkit.org/attachment.cgi?id=390168&action=review




--- Comment #28 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 390168
  --> https://bugs.webkit.org/attachment.cgi?id=390168
Patch

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

r=me with suggestions.

> Source/WTF/ChangeLog:13
> +	   small tables which capacity is <= 1024 based on collected data by
micro-benchmarking.

Please add a comment about why you chose to do this.

> Source/WTF/ChangeLog:22
> +	   * wtf/HashTraits.h:

Please remove.

> Source/WTF/wtf/HashTable.h:1260
> +	       double maxLoadRate = loadFactor;
> +	       double minLoadRate = 1.0 / minLoad;
> +	       double averageLoadRate = (maxLoadRate + minLoadRate) / 2;
> +	       double halfWayBetweenAverageAndMaxLoadRate = (averageLoadRate +
maxLoadRate) / 2;
> +	       return keyCount >= tableSize *
halfWayBetweenAverageAndMaxLoadRate;

nit: I suggest using "Ratio" instead of "Rate".  "Rate" makes me think of
frequency, which doesn't apply here.  Instead, what you're computing here is
the ratio of a load threshold relative to the tableSize.  So, I think "Ratio"
accurately describes the quantities you're computing here.

> Source/WTF/wtf/HashTraits.h:56
> +
> +    static constexpr unsigned maxLoadNumerator = 1;
> +    static constexpr unsigned maxLoadDenominator = 2;

As you mention offline, this will be removed.