User:Jminta:Sunbird Speed

From MozillaWiki
Jump to: navigation, search

Notes about places I've found where we can improve Sunbird's speed for 0.3a2 and beyond. (This is almost all front-end stuff, because that's what I know better.) Also includes some memory usage stuff.

Startup

  • The console spew I see tends to make me think we're creating 2 calCalendarManagers. Are we? Maybe someone is calling createInstance?
  • prepareCalendarUnifinder() creates the unifinder tree, then refreshEventTree() is called in update_date(), meaning we build the tree twice
  • gCalendarWindow constructor is huge, but new views will help a lot here

Trees

The unifinder (if we keep it?) and the task list code are almost identical. We might want to consider moving this to a single prototype, to reduce runtime memory usage. The following notes are all written in reference to the task list, but also apply to the unifinder.

I have a patch for all of this, but it requires substantial re-construction of the functions handling the trees. That is, it's quite high risk. Preliminary testing on the infamous 1200 event calendar does show ~40% select all time improvement, ~20% tree-rebuild time improvement (But we hardly ever rebuild the tree anymore), and marginal search time improvement. We also help our mouseover-previews.

  • The task list should be smart enough to incrementally rebuild itself (invalidateRow()) based on the information passed into onAddItem, onModifyItem, and onDeleteItem.
  • getRowOfCalendarTask(calTask) iterates through the entire tree. We call this function many times, especially for selection in the event-unifinder.
    • Because calTask items can mutate, it's not possible to simply take .indexOf on gTaskArray.
    • We should keep a second array of only event.id's. This we can do .indexOf on, and it's fast. The trick is to make sure that any time we sort gTaskArray, we keep this new array in sync.
  • Every time we rebuild the tree, we create a new calendarTaskView() object. We can definitely do without this.
  • compareTasks(a,b) has to compute the modifier each time. We should just ignore the modifier and use .reverse() if we later determine we wanted it
  • compareTasks(a,b) evaluates the switch() statement each time. Let's have many smaller sorters, and switch prior to calling .sort() to prevent this.
  • We're still trying to be 1.7 compatible. Once the new views go in, this won't be possible, and we should remove the checks here.

Unifinder only

All this is also included in the high-risk overhaul I put together. However, we should reconsider the whole unifinder from scratch probably, before considering landing any of it. I'm curious what the common use cases are for the unifinder.

  • The selection handlers are a mess. We add/remove extra event listeners we don't need. Also we have stuff in timeouts that shouldn't need to be there.
  • For searching, we get all the events via onGetItems, then iterate through them again after onOperationComplete to determine if they match our criteria. We should check them as soon as we get them.
  • We recompute the bounds of our search window (ie. next 7/14/31 days) before each calling of getItems. We should only calculate this when it changes.
  • Don't rebuild if the event that was changed is outside the displayed date range.

New Views

  • Depending on how the war-on-boxes goes, we might get hurt a bit here.
  • Don't worry about selecting events in views that aren't shown. (We'll need to redo calendarEventSelection to make this possible)
  • I'm showing about ~30% less memory usage during normal idle with the new views. I think our actual draw time is slower, though. (Hoping to look into this soon.) What other tests can we do?
  • What about saving the last 5(?) event lists (or even DOMs?) for each view, a la bfcache? I don't think many users navigate across a huge ranges of these. We could update these incrementally as well (done), and only need to redraw when we encounter an 'unusual' view.
    • Maybe not day/week, but month/multiweek could gain a lot.

Other

  • Kill some global variables (these are marginally faster, but use memory. I've been told it's better to lose them.)
    • prefService
    • strBundleService
  • Remove all those dump() and debug() calls in applicationUtils()
  • Actually, remove, dump()/debug() almost everywhere, except in catch(). If we want to examine something, we should add our own dump()s locally. Users never even see this.
  • I don't like the look of the way calendarEventSelection does getElementsByAttribute("disabledwhennoeventsareselected"), but I haven't done any testing here yet. Look into using a broadcaster.

Hopefully more coming...