Bug 75821 – Using strncmp() for comparing scheme and port numbers is inefficient

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Benjamin Poulain]

In KURL, we frequently compare very short strings, like the scheme and port.
Currently, we use strncmp(), we could just compare char by char and let the compiler inline everything.

[Benjamin Poulain]

Created attachment 121613 [details]
Patch

[Darin Adler]

Comment on attachment 121613 [details]
Patch

You should be able to do all of this without hard-coding the lengths of the arrays.

template<size_t bLength> static inline bool equal(const char* a, const char b[bLength])

Try it.

[Benjamin Poulain]

Comment on attachment 121613 [details]
Patch

I'll update later this week, I am busy with another bug atm :(

[Benjamin Poulain]

Created attachment 122562 [details]
Patch

[Benjamin Poulain]

The template "template<size_t bLength> static inline bool equal(const char* a, const char b[bLength])" never matched.
I read online to use a reference to the array, and this time the compiler match the template.

[Benjamin Poulain]

Created attachment 124046 [details]
Patch

[Darin Adler]

Comment on attachment 124046 [details]
Patch

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

> Source/WebCore/platform/KURL.cpp:1079
> +template<size_t referenceLength>
> +static inline bool equal(const char* str, size_t length, const char (&reference)[referenceLength])
> +{
> +    return length == referenceLength && equal(str, reference);
>  }

This compares two strings. It seems a little strange that the first string is named “str” with length “length” and the second is named “reference” with name “referenceLength”. I would just name them “a” and “b”, or “stringA” and “stringB”, and “lengthA” and “lengthB”, or something like that.

[Benjamin Poulain]

> This compares two strings. It seems a little strange that the first string is named “str” with length “length” and the second is named “reference” with name “referenceLength”. I would just name them “a” and “b”, or “stringA” and “stringB”, and “lengthA” and “lengthB”, or something like that.

Stricto sensu, it is comparing the string to an (reference) const array, that is why I had chosen the name "reference". I do not mind updating that :)

Thanks for the review!

[Anders Carlsson]

For reference, I still don't think it's necessary to declare the strings as character arrays, just make equals handle the trailing null character.

[Benjamin Poulain]

Committed r106025: <http://trac.webkit.org/changeset/106025>

[Darin Adler]

(In reply to comment #9)
> For reference, I still don't think it's necessary to declare the strings as character arrays, just make equals handle the trailing null character.

Right, we could easily do it that way which would look a little nicer.

[Benjamin Poulain]

Comment on attachment 124046 [details]
Patch

Clearing useless flag.