The default bug view has changed. See this FAQ.

Separate TreeContext and BytecodeEmitter

RESOLVED FIXED in mozilla15

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
mozilla15
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 621813 [details] [diff] [review]
patch

BytecodeEmitter ("BCE") is a sub-class of TreeContext ("TC").  This is
because parsing and bytecode generation share some state.  But the amount of
shared state is much smaller than TC;  i.e. lots of Parser-only state is
unnecessarily exposed to BCE.

This patch pulls out this shared state into a new class called
"SharedContext".

So, when generating bytecode, at the top-level we used to have a single BCE
object, which was also a TC (due to the inheritance);  the Parser pointed to
the TC sub-object.  With this patch, at the top-level we have a TC object
that the Parser points to, a BCE object (which points to the Parser object
but not to the TC object), and a SharedContext object that both the TC object
and the BCE object point to.

SharedContext::flags still has an ugly mixture of state (e.g. some of it is
Parser-only;  some is shared between Parser and BCE);  I plan to split that
up in a follow-up.

Features of the patch:

- TreeContext.h has the key data structure changes.

- Much of it is boring;  lots of converting |tc| to |tc->sc| and |bce| to
  |bce->sc|.

- Some places where we used to pass a TC or a BCE we now just pass a
  SharedContext.  Information hiding FTW!

- One nice thing is that InitBehavior, which was a gross hack,
  is no longer necessary.

- Another nice thing is that the TC-to-BCE down-cast in parentBCE() is no
  longer necessary.

- This patch is on top of the patch in bug 750606 but should be pretty
  comprehensible in isolation.
Attachment #621813 - Flags: review?(bhackett1024)
(Assignee)

Updated

5 years ago
Blocks: 752793
(Assignee)

Comment 1

5 years ago
Created attachment 622608 [details] [diff] [review]
rebased patch

Rebased patch to account for changes in the precursor patches, all of which have now landed.

bhackett, I promise I won't ask you for a front-end refactoring review for quite some time after this :)
Attachment #621813 - Attachment is obsolete: true
Attachment #621813 - Flags: review?(bhackett1024)
Attachment #622608 - Flags: review?(bhackett1024)
Attachment #622608 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 2

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f8305c48063e
https://hg.mozilla.org/mozilla-central/rev/f8305c48063e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.