1394719 - Optimize HasRTLChars usage

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[Olli Pettay [:smaug][[e-mail]]]

http://searchfox.org/mozilla-central/rev/cd82cacec2cf734768827ff85ba2dba90a534c5e/dom/base/nsGenericDOMDataNode.cpp#337-359
is super inefficient.
We end up copying data currently twice, since TextFragment copies too and we also do RTL scanning for the whole text even though it should be enough to do it only for the new text.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: insert_text_to_text_node.diff

When typing in middle of some <input> or <textarea> better to copy memory less and also RTL detection can be faster.
Depends on bug 1393232 
 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=9786b025f6338a5d009805cf4b3c2ebd55b40cb6

Comment 2

[(no longer active)]

Comment on attachment 8902225 [details] [diff] [review]
insert_text_to_text_node.diff

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

::: dom/base/nsGenericDOMDataNode.cpp
@@ +342,3 @@
>      // Allocate new buffer
>      int32_t newLength = textLength - aCount + aLength;
> +    nsString to;

Any reason not to use an nsAutoString here?  It may save on some allocations for small strings...

@@ +365,2 @@
>      bool ok =
> +      mText.SetTo(to, false, use2b);

Nit: one line please.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

(In reply to :Ehsan Akhgari (needinfo please, extremely long backlog) from comment #2)
> 
> Any reason not to use an nsAutoString here?  It may save on some allocations
> for small strings...
Because I want string buffer here to avoid copying then in nsTextFragment. I'll add a comment.

> 
> @@ +365,2 @@
> >      bool ok =
> > +      mText.SetTo(to, false, use2b);
> 
> Nit: one line please.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: insert_text_to_text_node_2.diff

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9931b3249b03
Optimize HasRTLChars usage and reduce malloc/free, r=ehsan

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9931b3249b03

Comment 7

[Itiel]

Could this patch cause the following crashes?
https://crash-stats.mozilla.com/report/index/f5b7ce8c-c508-441d-9393-cbe890170903
https://crash-stats.mozilla.com/report/index/e7d157b8-4348-446f-8298-ebeb10170902

This started right after this bug has landed, and the crash happens after clicking the prompt to update Nightly. Even with Nightly crashing, Nightly still starts with the updated version.
Using Windows 10 x86, latest Nightly, Hebrew locale.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

I don't see anything in the stack traces hinting that this could have caused those crashes. 
Could you please file a new bug, Core: Networking Cache, I think