[HTML5] Add speculative parsing to the HTML5 parser

RESOLVED FIXED

Status

()

Core
HTML: Parser
P1
normal
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: hsivonen, Assigned: hsivonen)

Tracking

Other Branch
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

9 years ago
When a script loads synchronously, the HTML5 parser should speculatively start parsing the rest of the network stream.
(Assignee)

Updated

9 years ago
Priority: -- → P1
(Assignee)

Updated

9 years ago
Depends on: 483158
(Assignee)

Updated

9 years ago
Duplicate of this bug: 503292
(Assignee)

Updated

9 years ago
Blocks: 515610
(Assignee)

Comment 3

8 years ago
Created attachment 405479 [details] [diff] [review]
First iteration
(Assignee)

Updated

8 years ago
Blocks: 521764
(Assignee)

Comment 4

8 years ago
Created attachment 406449 [details] [diff] [review]
Refresh patch
Attachment #405479 - Attachment is obsolete: true
(Assignee)

Comment 5

8 years ago
Created attachment 406988 [details] [diff] [review]
Use a method instead of copy&paste
Attachment #406449 - Attachment is obsolete: true
(Assignee)

Comment 6

8 years ago
Created attachment 407026 [details] [diff] [review]
Unrot after changes earlier in queue
Attachment #406988 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Attachment #407026 - Flags: review?(bnewman)
(Assignee)

Updated

8 years ago
Status: NEW → ASSIGNED
+// Not using macros for AddRef and Release in order to be able to refcount on
+// and create on different threads.
+
+nsrefcnt
+nsHtml5UTF16Buffer::AddRef()
+{

What's wrong with NS_IMPL_THREADSAFE_{AddRef,Release}?
(Assignee)

Comment 8

8 years ago
(In reply to comment #7)
> +// Not using macros for AddRef and Release in order to be able to refcount on
> +// and create on different threads.
> +
> +nsrefcnt
> +nsHtml5UTF16Buffer::AddRef()
> +{
> 
> What's wrong with NS_IMPL_THREADSAFE_{AddRef,Release}?

This object doesn't really need refcounting from multiple threads concurrently. Therefore, NS_IMPL_THREADSAFE_{AddRef,Release} would add an unnecessary atomic operation. I wanted to avoid adding an unnecessary atomic operation, because I thought it would be bad for CPU caches on multicores.

The thread-unsafe macro variants don't work, because they want the object to have a single owner thread. Therefore, they prevent the hand-off of objects between threads. It seems to me that letting the buffer objects assert about their owner thread and going in an manually changing the owner thread as appropriate would be an overkill, since the buffers are owner by nsHtml5StreamParser and protected by mTokenizerMutex and the parser would already be horribly broken if mTokenizerMutex weren't doing the right thing or used right.
(Assignee)

Comment 9

8 years ago
Created attachment 408855 [details] [diff] [review]
Remove printfs, make script running tree op initialization type-safe

I'm seeing a crash on tp4. I've tried sending various printfs to talos to figure out what's going wrong, but I have failed to get useful data. I suggest dealing with the tp4 crash as a follow-up bug, since this code doesn't run on talos by default.
Attachment #407026 - Attachment is obsolete: true
Attachment #408855 - Flags: review?(bnewman)
Attachment #407026 - Flags: review?(bnewman)
Comment on attachment 408855 [details] [diff] [review]
Remove printfs, make script running tree op initialization type-safe

Patch makes sense to me.

Definitely file a followup bug for the crash.  I think I might be seeing the same thing, and it does not appear related to speculative parsing.
Attachment #408855 - Flags: review?(bnewman) → review+
(Assignee)

Comment 11

8 years ago
Thank you for the review.

Landed as http://hg.mozilla.org/mozilla-central/rev/e04af661ed40
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Updated

7 years ago
Depends on: 645998
Landed followup to fix no-newline-at-end-of-file build warning on maemo (spammed for every .cpp file that includes nsHtml5Speculation.h):
  http://hg.mozilla.org/integration/mozilla-inbound/rev/94f56bd00f54
sample of warning:
{
In file included from /builds/slave/m-in-lnx-maemo5-gtk/build/parser/html/nsHtml5StreamParser.h:54,
                 from /builds/slave/m-in-lnx-maemo5-gtk/build/parser/html/nsHtml5Parser.h:61,
                 from /builds/slave/m-in-lnx-maemo5-gtk/build/parser/html/nsHtml5TreeBuilder.h:46,
                 from /builds/slave/m-in-lnx-maemo5-gtk/build/parser/html/nsHtml5AttributeName.cpp:50:
/builds/slave/m-in-lnx-maemo5-gtk/build/parser/html/nsHtml5Speculation.h:103:33: warning: no newline at end of file
}
http://tinderbox.mozilla.org/showlog.cgi?log=Mozilla-Inbound/1312310251.1312310578.31642.gz

(noticed this warning when pushing a bustage fix for bug 675499)
You need to log in before you can comment on or make changes to this bug.