Fix two byte string-search on big endian platforms

Fix two byte string-search on big endian platforms

Use AlignDown instead of IsAligned to avoid false negatives
on big endian platforms
Use byte with highest value to speedup search

BUG=

Committed: https://crrev.com/0e0c802858a432fa15afbdd8fcc17bac318791d9
Cr-Commit-Position: refs/heads/master@{#30615}

[skomski, 2015-09-05 09:35:08]

karl@skomski.com changed reviewers:
+ jkummerow@chromium.org

[skomski, 2015-09-05 09:45:58]

karl@skomski.com changed reviewers:
+ ishell@google.com

[skomski, 2015-09-05 09:45:58]

Sorry for all the trouble. This should get the mips build bots green again.

[skomski, 2015-09-05 10:13:23]

karl@skomski.com changed reviewers:
+ ishell@chromium.org
- ishell@google.com

[Jakob Kummerow, 2015-09-05 10:26:58]

https://codereview.chromium.org/1324803003/diff/1/src/string-search.h
File src/string-search.h (right):

https://codereview.chromium.org/1324803003/diff/1/src/string-search.h#newcode214
src/string-search.h:214: Max(static_cast<uint8_t>(pattern_first_char & 0xFF),
Why this change? "... & 0xFF" gives the low byte regardless of endian-ness, and
always searching for the low byte should be fine.

[skomski, 2015-09-05 10:42:27]

https://codereview.chromium.org/1324803003/diff/1/src/string-search.h
File src/string-search.h (right):

https://codereview.chromium.org/1324803003/diff/1/src/string-search.h#newcode214
src/string-search.h:214: Max(static_cast<uint8_t>(pattern_first_char & 0xFF),
On 2015/09/05 10:26:58, Jakob wrote:
> Why this change? "... & 0xFF" gives the low byte regardless of endian-ness,
and
> always searching for the low byte should be fine.

A lot faster since the low byte can be 0 and faster to search highest value byte

old                                     new

single character                        single character
Κ found at 922                          Κ found at 922
616                                     577
㎡ found at 13217                       ㎡ found at 13217
4931                                    4707
က found at 4096                         က found at 4096
9836                                    1634
＀ found at 65280                        ＀ found at 65280
36149                                   26415
ﾥ found at 65445                        ﾥ found at 65445
36666                                   31258
  found at 8197                           found at 8197
11757                                   3099
倂 found at 20482                       倂 found at 20482
17193                                   8780
                                        
linear search                           linear search
ΚΛ found at 922                         ΚΛ found at 922
504                                     565
㎡㎢ found at 13217                     ㎡㎢ found at 13217
5119                                    5014
ﾥﾦ found at 65445                       ﾥﾦ found at 65445
35496                                   32745
                                        
linear + bmh search                     linear + bmh search
ΚΛΜΝΞΟΠΡ found at 922                   ΚΛΜΝΞΟΠΡ found at 922
522                                     533
ﾥﾦﾧﾨﾩﾪﾫﾬ found at 65445                 ﾥﾦﾧﾨﾩﾪﾫﾬ found at 65445
35283                                   30513

[Jakob Kummerow, 2015-09-05 11:06:47]

> A lot faster since the low byte can be 0 and faster to search highest value
byte

But that depends on the input, right? For maximum speed you want to search for
the most *unique* byte in the string, but you can't determine that without
analyzing the string first. It's easy to construct examples where either of the
strategies "low byte", "lower-value byte", "high byte", "higher-value byte" gets
pathologically unlucky. Example: "higher-value byte" will be slow here:
subject: 0xFF01, 0xFF02, 0xFF03, ..., 0xFFFE, 0x12FF
pattern: 0x12FF

So in the absence of any data about typical distributions of byte values of
two-byte strings, I would have gone for the simpler code.

LGTM anyway.

[skomski, 2015-09-05 14:08:13]

The CQ bit was checked by karl@skomski.com

[commit-bot: I haz the power, 2015-09-05 14:08:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1324803003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1324803003/1

[commit-bot: I haz the power, 2015-09-05 14:10:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-05 14:10:44]

Try jobs failed on following builders:
  v8_win_compile_dbg on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/7416)
  v8_win_nosnap_shared_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_compil...)

[skomski, 2015-09-05 18:14:09]

Fixed the windows warning and simplified the code a bit in the process because
from some benchmarks it results in the same or even better performance.

[Jakob Kummerow, 2015-09-07 08:42:57]

The CQ bit was checked by jkummerow@chromium.org

[Jakob Kummerow, 2015-09-07 08:42:57]

lgtm

[commit-bot: I haz the power, 2015-09-07 08:43:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1324803003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1324803003/20001

[commit-bot: I haz the power, 2015-09-07 09:07:51]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-09-07 09:08:13]

Patchset 2 (id:??) landed as
https://crrev.com/0e0c802858a432fa15afbdd8fcc17bac318791d9
Cr-Commit-Position: refs/heads/master@{#30615}

[paul.l..., 2015-09-07 16:56:42]

Thanks a lot for the big-endian fix, Karl, we really appreciate it!