Ticket #5393 (closed defect: fixed)

Opened 7 years ago

Last modified 6 years ago

journal updates are too heavy, should happen less often

Reported by: tomeu Owned by: tomeu
Priority: high Milestone: 8.2.0 (was Update.2)
Component: journal-activity Version:
Keywords: Cc: marco, mtd
Action Needed: Verified: no
Deployments affected: Blocked By:
Blocking:

Description

Right now, the journal listens for changes in the DS and updates itself every time there's a create/update/delete.

Updating the list view is quite expensive and is slowing those operations that need to update data in the DS. See tickets #5344, #5228 and #5235.

Instead, when we are notified of a change,we should just invalidate the cache, and when the journal becomes active, update the UI.

Switching to the journal will be slower, but at least we won't be degrading general performance of sugar and activities.

Attachments

update_on_visible.patch (15.9 kB) - added by tomeu 7 years ago.
update_on_visible.2.patch (11.2 kB) - added by tomeu 7 years ago.
0001-Update-only-when-visible.patch (11.2 kB) - added by tomeu 6 years ago.

Change History

Changed 7 years ago by jg

  • keywords Update.1? removed
  • priority changed from normal to high
  • milestone changed from Never Assigned to Update.1

Changed 7 years ago by tomeu

Changed 7 years ago by tomeu

  • keywords review+ added
  • milestone changed from Update.1 to Retriage, Please!

The patch is much bigger than I expected, and couldn't measure dramatic improvements in performance.

I think this should go in Update.2.

Changed 7 years ago by tomeu

From #5228, activity startup time:

Without ipython + with pulsing at 5fps (instead of 10fps) + with journal patch #5393: 5.59s.

Changed 7 years ago by jg

  • milestone changed from Retriage, Please! to Update.2

Changed 7 years ago by tomeu

  • cc marco added
  • milestone changed from Update.2 to Retriage, Please!

Jim, sorry for moving this ticket back and forth.

Today I've been measuring activity startup and Marco and Chris have agreed with me that we should try to put this in joyride before continuing analysis.

I'm working now in simplifying this patch, so we can better review it and assure it has no unintended consequences.

Changed 7 years ago by tomeu

Changed 7 years ago by tomeu

  • keywords review? added; review+ removed

Attached a bit simpler patch.

Changed 7 years ago by jg

  • milestone changed from Retriage, Please! to Update.2

Unless you have high confidence in this, it seems that the gain (if any) should wait for update.2.

Changed 6 years ago by mtd

  • cc mtd added

Changed 6 years ago by marco

Is the patch on master? Otherwise I guess it should be sent for review.

Changed 6 years ago by tomeu

Changed 6 years ago by tomeu

  • keywords review? removed
  • status changed from new to closed
  • resolution set to fixed

In git.

Note: See TracTickets for help on using tickets.