[Idle time spell checker] Add paragraph-level heuristic to hot mode requester

[Idle time spell checker] Add paragraph-level heuristic to hot mode requester

This patch modifies how idle time spell checker decides checking range in
hot mode.

Before:
- If the total text length in the editable is at most 1024, check everything
- Otherwise, check a chunk of text of length 1024, expanded to sentence
  boundaries, centered at the reference position

After:
- If the total text length in the editable is at most 128, check everything
- Otherwise, if the reference position is in a paragraph of length at most
  1024, check the paragraph
- Otherwise, check a chunk of text of length 1024, expanded to sentence
  boundaries, centered at the reference position

BUG=517298
Review-Url: https://codereview.chromium.org/2867393003
Cr-Commit-Position: refs/heads/master@{#471012}
Committed: https://chromium.googlesource.com/chromium/src/+/5a291d60a6ace6547d1653ffa531e7e12cdc9725

[Xiaocheng, 2017-05-09 22:18:19]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 22:19:57]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xiaocheng, 2017-05-09 22:24:28]

Description was changed from

==========
[Idle time spell checker] Add paragraph-level heuristic to hot mode requester

BUG=
==========

to

==========
[Idle time spell checker] Add paragraph-level heuristic to hot mode requester

This patch modifies how idle time spell checker decides checking range in
hot mode.

Before:
- If the total text length in the editable is at most 1024, check everything
- Otherwise, check a chunk of text of length 1024, expanded to sentence
  boundaries, centered at the reference position

After:
- If the total text length in the editable is at most 128, check everything
- Otherwise, if the reference position is in a paragraph of length at most
  1024, check the paragraph
- Otherwise, check a chunk of text of length 1024, expanded to sentence
  boundaries, centered at the reference position

BUG=517298
==========

[commit-bot: I haz the power, 2017-05-09 23:12:24]

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

[commit-bot: I haz the power, 2017-05-09 23:12:24]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[Xiaocheng, 2017-05-10 00:28:07]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-10 00:29:12]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xiaocheng, 2017-05-10 01:03:27]

xiaochengh@chromium.org changed reviewers:
+ yosin@chromium.org

[Xiaocheng, 2017-05-10 01:03:27]

PTAL.

[commit-bot: I haz the power, 2017-05-10 02:31:24]

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

[commit-bot: I haz the power, 2017-05-10 02:31:25]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet;
builder either lacks capacity or does not exist (misspelled?))
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build
has not started yet; builder either lacks capacity or does not exist
(misspelled?))
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))

[Xiaocheng, 2017-05-10 02:39:56]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-10 02:40:47]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yosin_UTC9, 2017-05-10 02:44:05]

I think idle time spell checker, ITSC is too aggressive to check text.
We don't need to check text so frequently. In other words, number of
request of ITSC should be less than sync spell check, SSC, every text length.

Number of request time of SSC means number of text changes and it is too
frequent
to check. We want to reduce this number because checking result will be obsolete
after each text modification.

ITSC reduces such obsolete result.

According to [1], the average rate for words per minute is 33. If each word is 
7 characters, users types 33 x 7 = 231 chars in minute == 3.85 char in second
== 259ms for one chars.

This means requesting every 259ms is enough for average users. For professional
typist WPM is 50 to 80[1], so 100ms is enough for professional.

BTW, aggregation of number of request covers composing text, this may be typical
use. But, we need to consider about number of request of each chunk.


[1] https://en.wikipedia.org/wiki/Words_per_minute

[Xiaocheng, 2017-05-10 03:09:42]

On 2017/05/10 at 02:44:05, yosin wrote:
> I think idle time spell checker, ITSC is too aggressive to check text.
> We don't need to check text so frequently. In other words, number of
> request of ITSC should be less than sync spell check, SSC, every text length.
> 
> Number of request time of SSC means number of text changes and it is too
frequent
> to check. We want to reduce this number because checking result will be
obsolete
> after each text modification.
> 
> ITSC reduces such obsolete result.
> 
> According to [1], the average rate for words per minute is 33. If each word is

> 7 characters, users types 33 x 7 = 231 chars in minute == 3.85 char in second
> == 259ms for one chars.
> 
> This means requesting every 259ms is enough for average users. For
professional
> typist WPM is 50 to 80[1], so 100ms is enough for professional.
> 
> BTW, aggregation of number of request covers composing text, this may be
typical
> use. But, we need to consider about number of request of each chunk.
> 
> 
> [1] https://en.wikipedia.org/wiki/Words_per_minute

This is a different issue. There is no conflict between reducing request
frequency and reducing length of each request.

This patch focuses on the latter.

Regarding request frequency, I think we should add a metric first.

[yosin_UTC9, 2017-05-10 06:05:10]

On 2017/05/10 at 03:09:42, xiaochengh wrote:
> On 2017/05/10 at 02:44:05, yosin wrote:
> > I think idle time spell checker, ITSC is too aggressive to check text.
> > We don't need to check text so frequently. In other words, number of
> > request of ITSC should be less than sync spell check, SSC, every text
length.
> > 
> > Number of request time of SSC means number of text changes and it is too
frequent
> > to check. We want to reduce this number because checking result will be
obsolete
> > after each text modification.
> > 
> > ITSC reduces such obsolete result.
> > 
> > According to [1], the average rate for words per minute is 33. If each word
is 
> > 7 characters, users types 33 x 7 = 231 chars in minute == 3.85 char in
second
> > == 259ms for one chars.
> > 
> > This means requesting every 259ms is enough for average users. For
professional
> > typist WPM is 50 to 80[1], so 100ms is enough for professional.
> > 
> > BTW, aggregation of number of request covers composing text, this may be
typical
> > use. But, we need to consider about number of request of each chunk.
> > 
> > 
> > [1] https://en.wikipedia.org/wiki/Words_per_minute
> 
> This is a different issue. There is no conflict between reducing request
frequency and reducing length of each request.
> 
> This patch focuses on the latter.
> 
> Regarding request frequency, I think we should add a metric first.

lgtm for this patch.

But, I need to understand efficiency of idle-time-spellchecker regarding number
of requests.

[yosin_UTC9, 2017-05-10 06:05:15]

The CQ bit was unchecked by yosin@chromium.org

[yosin_UTC9, 2017-05-10 06:05:18]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2017-05-10 06:05:27]

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

[commit-bot: I haz the power, 2017-05-10 06:05:28]

This CL has an open dependency (Issue 2869213002 Patch 1). Please resolve the
dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[Xiaocheng, 2017-05-10 09:04:37]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-10 09:05:08]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 11:53:47]

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

[commit-bot: I haz the power, 2017-05-10 11:53:48]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Xiaocheng, 2017-05-10 17:06:33]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-10 17:07:54]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 19:15:25]

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

[commit-bot: I haz the power, 2017-05-10 19:15:26]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Xiaocheng, 2017-05-11 06:55:35]

The CQ bit was checked by xiaochengh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 06:56:10]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-11 09:18:30]

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

[commit-bot: I haz the power, 2017-05-11 09:18:30]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2017-05-11 18:11:09]

The CQ bit was checked by xiaochengh@chromium.org

[commit-bot: I haz the power, 2017-05-11 18:12:32]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-11 18:18:00]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1494526269860440, "parent_rev":
"2a26cfe6edba4fa96e7179933392e085f1052d30", "commit_rev":
"5a291d60a6ace6547d1653ffa531e7e12cdc9725"}

[commit-bot: I haz the power, 2017-05-11 18:18:15]

Description was changed from

==========
[Idle time spell checker] Add paragraph-level heuristic to hot mode requester

This patch modifies how idle time spell checker decides checking range in
hot mode.

Before:
- If the total text length in the editable is at most 1024, check everything
- Otherwise, check a chunk of text of length 1024, expanded to sentence
  boundaries, centered at the reference position

After:
- If the total text length in the editable is at most 128, check everything
- Otherwise, if the reference position is in a paragraph of length at most
  1024, check the paragraph
- Otherwise, check a chunk of text of length 1024, expanded to sentence
  boundaries, centered at the reference position

BUG=517298
==========

to

==========
[Idle time spell checker] Add paragraph-level heuristic to hot mode requester

This patch modifies how idle time spell checker decides checking range in
hot mode.

Before:
- If the total text length in the editable is at most 1024, check everything
- Otherwise, check a chunk of text of length 1024, expanded to sentence
  boundaries, centered at the reference position

After:
- If the total text length in the editable is at most 128, check everything
- Otherwise, if the reference position is in a paragraph of length at most
  1024, check the paragraph
- Otherwise, check a chunk of text of length 1024, expanded to sentence
  boundaries, centered at the reference position

BUG=517298

Review-Url: https://codereview.chromium.org/2867393003
Cr-Commit-Position: refs/heads/master@{#471012}
Committed:
https://chromium.googlesource.com/chromium/src/+/5a291d60a6ace6547d1653ffa531...
==========

[commit-bot: I haz the power, 2017-05-11 18:18:15]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/5a291d60a6ace6547d1653ffa531...