Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(30)

Issue 328453003: Refactor to remove unnecessary code from the string hashing functions (Closed)

Created:
6 years, 6 months ago by atreat
Modified:
6 years, 6 months ago
CC:
blink-reviews, blink-reviews-wtf_chromium.org, Mikhail, abarth-chromium
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Refactor to remove unnecessary code from the string hashing functions Previously, StringHasher contained methods for calculating the hash of a pure C character array without having to pass in the length of the array. This was only used for AtomicStrings constructed via a string literal. The impl of the specific hashing function currently used in WTF already checks for null terminated character array and when the final StringImpl* for the AtomicString is constructed we do so again. This patch just computes the length up front and passes that information along to the hashing function. Eventually this length is used to construct the StringImpl without computing again. BUG=none Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175826

Patch Set 1 #

Patch Set 2 : Fix the unit tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -122 lines) Patch
M Source/wtf/StringHasher.h View 4 chunks +0 lines, -57 lines 0 comments Download
M Source/wtf/StringHasherTest.cpp View 1 14 chunks +9 lines, -45 lines 0 comments Download
M Source/wtf/text/AtomicString.cpp View 2 chunks +1 line, -20 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
atreat
PTAL
6 years, 6 months ago (2014-06-09 18:25:23 UTC) #1
atreat
Added some other reviewers since eseidel is away
6 years, 6 months ago (2014-06-09 20:09:17 UTC) #2
abarth-chromium
lgtm
6 years, 6 months ago (2014-06-09 22:01:56 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/adam.treat@samsung.com/328453003/20001
6 years, 6 months ago (2014-06-09 22:02:41 UTC) #4
commit-bot: I haz the power
6 years, 6 months ago (2014-06-09 22:14:06 UTC) #5
Message was sent while issue was closed.
Change committed as 175826

Powered by Google App Engine
This is Rietveld 408576698