636402 - Simplify Sync.js to avoid creating new objects

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Philipp von Weitershausen [:philikon]]

Sync.js can be much simpler and thus avoid creating as many objects as it does now for one invocation:

* expose a function to create a callback, e.g. Sync.makeCallback()
* expose a function to spin the event loop, e.g. Sync.waitForCallback()
* no longer call the async function, leave that up to the caller (func.apply(this, arguments) creates allocates new objects!)
* Sync.sleep() can be replaced by custom nsITimer based solutions. It's only used in SyncEngine._processIncoming anyway. That way we can also avoid creating nsITimer instances over and over.

I have a patch in the works.

Comment 1

[Philipp von Weitershausen [:philikon]]

Attachment: v1

This does basically what's described in comment 0:

* Sync() and Sync.withCb() are replaced by two helpers:

  a) Utils.makeSyncCallback() for creating the special callback
  b) Utils.waitForSyncCallback(cb) for spinning the event loop

  The observant reader may have noticed that this way we no longer wrap the
  async function in a closure but instead leave it up to the caller to
  explicitly call it and these two helpers. The idea here is to avoid
  accessing the 'arguments' object (which instantiates it... bad bad bad)

* Utils.makeSyncCallback() also creates less objects by stashing the state and
  return value directly onto the callback.

* Sync.sleep() is replaced with nsITimer based solutions on each engine. The
  benefit here is that we don't create a new nsITimer object on every single
  record (SyncEngine._processIncoming calls Sync.sleep(0) after each record.)

* Lots of places unnecessary imported Sync.js. Removed.

Baibai Sync.js (though the saga is far from over)

Comment 2

[Philipp von Weitershausen [:philikon]]

In my test profile with initially 4.5M created objects, this cuts the number of created objects by ca. 0.5M. Should block Fennec big time.

Comment 3

[Richard Newman [:rnewman]]

This is the first thing in my queue after breakfast. Starting the day right!

Comment 4

[Richard Newman [:rnewman]]

Comment on attachment 515014 [details] [diff] [review]
v1

Beautiful.

Comment 5

[Richard Newman [:rnewman]]

Thumbs up from local crossweave, btw.

Comment 6

[Philipp von Weitershausen [:philikon]]

Landed in fx-sync: https://hg.mozilla.org/services/fx-sync/rev/1fbce35f48a4

Comment 7

[Philipp von Weitershausen [:philikon]]

Merged to s-c: http://hg.mozilla.org/services/services-central/rev/0d03339c1895

Comment 8

[Philipp von Weitershausen [:philikon]]

Merged to m-c: http://hg.mozilla.org/mozilla-central/rev/0d03339c1895