957593 - txXSLTNumber::isAlphaNumeric should be using lookup tables

Status: RESOLVED FIXED

(Core :: XSLT, defect)

Description

[Nathan Froyd [:froydnj]]

isAlphaNumeric takes up ~8KB on ARM, ~11KB on x86-64.  Let's save a significant amount of space by using lookup tables instead.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: part 1 - eliminate TX_MATCH_CHAR

Matching a single character is the same as matching an inclusive range of
characters where the start and end are the same.  This is mostly a
mechanical patch so that part 2 is less cluttered.

Comment 2

[Nathan Froyd [:froydnj]]

Attachment: part 2 - implement txXSLTNumber::isAlphaNumeric with binary search for a codesize win

This patch saves ~5KB on ARM, ~7KB on x86-64.

It does make the (common?) ASCII case slower, but we could re-insert
explicit checks for the ASCII range if desired.  I don't know how many
people really care about XSLT performance in Firefox.

Comment 3

[Nathan Froyd [:froydnj]]

Comment on attachment 8357170 [details] [diff] [review]
part 1 - eliminate TX_MATCH_CHAR

comment 1 has details.

Comment 4

[Peter Van der Beken [:peterv]]

Comment on attachment 8357173 [details] [diff] [review]
part 2 - implement txXSLTNumber::isAlphaNumeric with binary search for a codesize win

Review of attachment 8357173 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/xslt/src/xslt/txXSLTNumber.cpp
@@ +731,5 @@
> +    const CharRange* element = std::lower_bound(&alphanumericRanges[0], end, search);
> +    if (element == end) {
> +        return false;
> +    }
> +    return element->lower >= ch && ch <= element->upper;

This seems wrong, it ssems to me it should be:

  return element->lower <= ch && ch <= element->upper;

Comment 5

[Nathan Froyd [:froydnj]]

Attachment: part 2 - implement txXSLTNumber::isAlphaNumeric with binary search for a codesize win

Ah, good catch, that was sloppy translation on my part.

Comment 6

[Nathan Froyd [:froydnj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1243b40c5da1
https://hg.mozilla.org/integration/mozilla-inbound/rev/f52e4fe3cf77

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1243b40c5da1
https://hg.mozilla.org/mozilla-central/rev/f52e4fe3cf77