1269490 - share atoms between the html parser and nsGkAtomList

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Nathan Froyd [:froydnj]]

We have a lot of duplicate atoms (and string buffers, etc.) that could be shared between the HTML5 parser and nsGkAtomList.  We should share those atoms, which would save ~50-60K of libxul size on a 64-bit platform, and make startup a wee bit faster by not having to intern everything.

Comment 2

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1269490 - Merge nsHtml5Atoms into nsGkAtoms.

Review commit: https://reviewboard.mozilla.org/r/131988/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/131988/

Comment 3

[Henri Sivonen (:hsivonen)]

Comment on attachment 8859927 [details]
Bug 1269490 - Merge nsHtml5Atoms into nsGkAtoms.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131988/diff/1-2/

Comment 4

[Henri Sivonen (:hsivonen)]

Attachment: Translator changes

Comment 5

[William Chen [:wchen]]

Comment on attachment 8859984 [details] [diff] [review]
Translator changes

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

::: translator-src/nu/validator/htmlparser/cpptranslate/CppTypes.java
@@ +146,5 @@
> +            String line;
> +            while ((line = atomReader.readLine()) != null) {
> +                manualAtoms.append(line);
> +                manualAtoms.append('\n');
> +                if (line.startsWith("// BEGIN GENERATED")) {

The list is generated atoms is long and the "// BEGIN GENERATED" warning block is far from the end of the file so it seems likely that someone will append to the middle or end of the file when adding a new atom. At the least we should add a comment at the end, something like "// END GENERATED ATOMS, DO NOT ADD CODE BELOW THIS LINE" and maybe also add a comment after each generated atoms saying "// ATOM GENERATED BY HTML PARSER TRANSLATOR" to make it even more clear so that we don't get atoms accidentally added to the middle of the list.

Alternatively, I think it would be even better if we generated the parser atoms to another file that we include in nsGkAtomList.h so then we don't have to worry about atoms manually added in the wrong place.

Comment 6

[William Chen [:wchen]]

Comment on attachment 8859927 [details]
Bug 1269490 - Merge nsHtml5Atoms into nsGkAtoms.

https://reviewboard.mozilla.org/r/131988/#review138668

Comment 7

[Henri Sivonen (:hsivonen)]

Comment on attachment 8859927 [details]
Bug 1269490 - Merge nsHtml5Atoms into nsGkAtoms.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/131988/diff/2-3/

Comment 8

[Henri Sivonen (:hsivonen)]

Attachment: Translator changes, v2

Comment 9

[William Chen [:wchen]]

Comment on attachment 8859927 [details]
Bug 1269490 - Merge nsHtml5Atoms into nsGkAtoms.

https://reviewboard.mozilla.org/r/131988/#review138980

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/355a23c6c2bd
Merge nsHtml5Atoms into nsGkAtoms. r=wchen

Comment 11

[Henri Sivonen (:hsivonen)]

https://hg.mozilla.org/projects/htmlparser/rev/9e783e21265f51e3ac96feca70dd025f03c3cf8e
Mozilla bug 1269490 - Merge nsHtml5Atoms into nsGkAtoms. r=wchen.

Comment 12

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/355a23c6c2bd