[webkit-reviews] review granted: [Bug 83968] Add a version of StringImpl::find() without offset : [Attachment 137194] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 83968: Add a version of StringImpl::find() without offset
https://bugs.webkit.org/show_bug.cgi?id=83968

Attachment 137194: Patch
https://bugs.webkit.org/attachment.cgi?id=137194&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=137194&action=review


> Source/JavaScriptCore/ChangeLog:11
> +	   This gives a 12% gains for strings of distribution of strings
between 30 and 100 characters.

I don't completely understand this sentence.

> Source/WTF/ChangeLog:11
> +	   By not havig the index, we can skip a couple of branches and a few
instructions. This gives

Typo: havig -> having.

> Source/WTF/ChangeLog:14
> +	   The case of empty string is moved bellow (matchLength == 1) since it
is a less common operation.

Typo: bellow -> below.	

How did you determine that finding on the empty string is less common?

> Source/WTF/wtf/text/StringImpl.cpp:901
> +    // Check for null or empty string to match against
> +    if (UNLIKELY(!matchString))
> +	   return notFound;

I don't think this checks for empty strings.