base/i18n: Class for efficiently searching the same query over many texts.

base/i18n: Class for efficiently searching the same query over many texts.

StringSearchIgnoringCaseAndAccents (implemented by ICU's usearch_* family)
takes significantly long time for preprocessing the query text. Thus, when
we want to query over multiple different texts with the same query,
it becomes much more efficient if we reuse the preprocessed query.

Here's the super micro benchmark result:

TEST(StringSearchTest, BenchmarkOld) {
  string16 query(ASCIIToUTF16("hello"));
  string16 text(ASCIIToUTF16("hello"));
  for (int i = 0; i < 1000000; ++i)
    StringSearchIgnoringCaseAndAccents(query, text, NULL, NULL);
}
TEST(StringSearchTest, BenchmarkNew) {
  FixedPatternStringSearchIgnoringCaseAndAccents query(ASCIIToUTF16("hello"));
  string16 text(ASCIIToUTF16("hello"));
  for (int i = 0; i < 1000000; ++i)
    query.Search(text, NULL, NULL);
}
[ RUN      ] StringSearchTest.BenchmarkOld
[       OK ] StringSearchTest.BenchmarkOld (68230 ms)
[ RUN      ] StringSearchTest.BenchmarkNew
[       OK ] StringSearchTest.BenchmarkNew (1282 ms)

BUG=255269
TEST=base_unittests --gtest_filter='StringSearchTest.*'

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=211052

[kinaba, 2013-06-28 03:35:00]

jshin@, could you take a look as the owner of base/i18n?

Please refer the associated BUG tracker for the context of this patch.

[kinaba, 2013-07-02 16:11:14]

friendly ping.

[jungshik at Google, 2013-07-09 19:12:50]

On 2013/07/02 16:11:14, kinaba wrote:
> friendly ping.

Sorry for the delay. I'll review this this afternoon.

[jungshik at Google, 2013-07-10 21:27:27]

On 2013/07/09 19:12:50, Jungshik Shin wrote:
> On 2013/07/02 16:11:14, kinaba wrote:
> > friendly ping.
> 
> Sorry for the delay. I'll review this this afternoon.

Can you address my comment above? In addition, you should add 'TEST' line.

[jungshik at Google, 2013-07-10 21:27:52]

Sorry that I forgot to publish my inline comment.

https://codereview.chromium.org/18031015/diff/1/base/i18n/string_search.cc
File base/i18n/string_search.cc (right):

https://codereview.chromium.org/18031015/diff/1/base/i18n/string_search.cc#ne...
base/i18n/string_search.cc:45: usearch_setText(search_, in_this.data(),
in_this.size(), &status);
You don't need to check for |search_| != NULL here. |usearch_setText| will do
the job for you and set |status| to U_ILLEGAL_ARGUMENT_ERROR.

[kinaba, 2013-07-11 03:14:22]

Updated.

https://codereview.chromium.org/18031015/diff/1/base/i18n/string_search.cc
File base/i18n/string_search.cc (right):

https://codereview.chromium.org/18031015/diff/1/base/i18n/string_search.cc#ne...
base/i18n/string_search.cc:45: usearch_setText(search_, in_this.data(),
in_this.size(), &status);
On 2013/07/10 21:27:52, Jungshik Shin wrote:
> You don't need to check for |search_| != NULL here. |usearch_setText| will do
> the job for you and set |status| to U_ILLEGAL_ARGUMENT_ERROR. 

Done. Thanks,

[jungshik at Google, 2013-07-11 06:23:01]

Thanks. LGTM 

On 2013/07/11 03:14:22, kinaba wrote:
> Updated.
> 
> https://codereview.chromium.org/18031015/diff/1/base/i18n/string_search.cc
> File base/i18n/string_search.cc (right):
> 
>
https://codereview.chromium.org/18031015/diff/1/base/i18n/string_search.cc#ne...
> base/i18n/string_search.cc:45: usearch_setText(search_, in_this.data(),
> in_this.size(), &status);
> On 2013/07/10 21:27:52, Jungshik Shin wrote:
> > You don't need to check for |search_| != NULL here. |usearch_setText| will
do
> > the job for you and set |status| to U_ILLEGAL_ARGUMENT_ERROR. 
> 
> Done. Thanks,

[commit-bot: I haz the power, 2013-07-11 06:34:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinaba@chromium.org/18031015/16001

[commit-bot: I haz the power, 2013-07-11 09:32:33]

Change committed as 211052