Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(247)

Issue 10240043: Lazily construct taming classes in Domado. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 9 months ago by kpreid2
Modified:
12 years, 9 months ago
Reviewers:
ihab.awad
CC:
google-caja-discuss_googlegroups.com
Base URL:
http://google-caja.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Defer the construction of taming ctors and their prototypes until an instance is going to be created, or the inert ctor global (e.g. window.Element) is accessed. This reduces the startup time of Domado, and eliminates nearly all of the time cost of supporting features which are not used by the guest content (e.g. XMLHttpRequest or <canvas>). This is also part of the refactoring-Domado project: the lazy loading enforces a lack of imperative dependencies between individual taming classes. Certain fundamental classes are not made lazy, as they would always be loaded and disentangling them would be additional work, such as Node, Element, Window, and Document. I expect to move these to the same lazy infrastructure as part of future refactoring. Supporting changes: * Events created by document.createEvent() are no longer a distinct type; the notYetDispatched flag turns out to be exactly sufficient (and secure) for the distinctions we need to make. * Newly visible-to-the-guest types: CSSStyleDeclaration (old laziness kludge didn't allow exporting) ImageData (now uses prototype inheritance) CajaComputedCSSStyleDeclaration (implementation detail) CajaFormField (implementation detail) * Added a REPL which evaluates inside the sandbox, for debugging (linked from test-index.html). * TameImageData now uses prototype inheritance. * Fixed scanner's check for taming ctors leaking having been broken by the move to a single SES frame. * Scanner is aware of newly exported CSSStyleDeclaration Performance notes: Measuring using console.time() in Chrome, approximately 130 ms is removed from the run time of attachDocument; this figure is adjusted for the cost of lazy loading that always happens, namely Event for onload events. Test run time changes (Firefox; one run, so very noisy). domado-global particularly benefits because all it does is start many trivial guests. guest-domado-global-es53-min 15.956/26.862 = 0.59 guest-domado-global-es5-min 3.519 /4.684 = 0.75 cajajs-invocation-nomin-es53 28.556/24.015 = 1.18 cajajs-invocation-nomin-es5 6.252 /6.298 = 0.99 cajajs-invocation-min-es53 24.473/22.089 = 1.10 cajajs-invocation-min-es5 6.656 /5.245 = 1.26 guest-domado-dom-es53-min 12.551/13.500 = 0.92 guest-domado-dom-es5-min 5.339 /5.117 = 1.04 @r5450

Patch Set 1 #

Total comments: 1

Patch Set 2 : Lazily construct taming classes in Domado. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+991 lines, -846 lines) Patch
M src/com/google/caja/plugin/domado.js View 1 45 chunks +931 lines, -827 lines 0 comments Download
M tests/com/google/caja/plugin/es53-test-scan-guest.js View 1 9 chunks +29 lines, -19 lines 0 comments Download
A tests/com/google/caja/plugin/repl.html View 1 chunk +29 lines, -0 lines 0 comments Download
M tests/com/google/caja/plugin/test-index.html View 1 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 3
kpreid2
12 years, 9 months ago (2013-06-12 19:19:02 UTC) #1
ihab.awad
lgtm++ https://codereview.appspot.com/10240043/diff/1/src/com/google/caja/plugin/domado.js File src/com/google/caja/plugin/domado.js (right): https://codereview.appspot.com/10240043/diff/1/src/com/google/caja/plugin/domado.js#newcode5788 src/com/google/caja/plugin/domado.js:5788: // It also matches browser behavior (Chrome and ...
12 years, 9 months ago (2013-06-17 18:32:27 UTC) #2
kpreid2
12 years, 9 months ago (2013-06-17 19:20:30 UTC) #3
Defer the construction of taming ctors and their prototypes until an
instance is going to be created, or the inert ctor global (e.g.
window.Element) is accessed.

This reduces the startup time of Domado, and eliminates nearly all of
the time cost of supporting features which are not used by the guest
content (e.g. XMLHttpRequest or <canvas>).

This is also part of the refactoring-Domado project: the lazy loading
enforces a lack of imperative dependencies between individual taming
classes.

Certain fundamental classes are not made lazy, as they would always
be loaded and disentangling them would be additional work, such as
Node, Element, Window, and Document. I expect to move these to the
same lazy infrastructure as part of future refactoring.

Supporting changes:
* Events created by document.createEvent() are no longer a distinct
  type; the notYetDispatched flag turns out to be exactly sufficient
  (and secure) for the distinctions we need to make.
* Newly visible-to-the-guest types:
    CSSStyleDeclaration (old laziness kludge didn't allow exporting)
    ImageData (now uses prototype inheritance)
    CajaComputedCSSStyleDeclaration (implementation detail)
    CajaFormField (implementation detail)
* Added a REPL which evaluates inside the sandbox, for debugging
  (linked from test-index.html).
* TameImageData now uses prototype inheritance.
* Fixed scanner's check for taming ctors leaking having been broken
  by the move to a single SES frame.
* Scanner is aware of newly exported CSSStyleDeclaration

Performance notes:

Measuring using console.time() in Chrome, approximately 130 ms is
removed from the run time of attachDocument; this figure is adjusted
for the cost of lazy loading that always happens, namely Event for
onload events.

Test run time changes (Firefox; one run, so very noisy). domado-global
particularly benefits because all it does is start many trivial guests.
guest-domado-global-es53-min    15.956/26.862 = 0.59
guest-domado-global-es5-min     3.519 /4.684  = 0.75
cajajs-invocation-nomin-es53    28.556/24.015 = 1.18
cajajs-invocation-nomin-es5     6.252 /6.298  = 0.99
cajajs-invocation-min-es53      24.473/22.089 = 1.10
cajajs-invocation-min-es5       6.656 /5.245  = 1.26
guest-domado-dom-es53-min       12.551/13.500 = 0.92
guest-domado-dom-es5-min        5.339 /5.117  = 1.04
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b