Opened 12 years ago

Closed 12 years ago

#1247 closed bug (fixed)

Tracker multiplies calls (open,stat) per open window

Reported by: mmu_man Owned by: axeld
Priority: low Milestone: R1
Component: Applications/Tracker Version: R1/pre-alpha1
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All


When volume usage bars are enabled, Tracker regularily (several times /s) forces recreating BVolume objects, implying multiple calls to open() and stat() on each volume's root folder. While it's needed only for the desktop window, it is actually done by each open window, so if you have 10 open windows you get 30 stat() per second, which on uncached network fs is quite wasteful. Also polutes syslog.

It was so at least in Zeta's Tracker last time I checked, didn't try with svn but I don't think it's ever been fixed.

Attachments (2)

volumebar.diff (7.8 KB) - added by mmlr 12 years ago.
Fixed patch that does not leak a BVolume and doesn't crash on volumes with no space bar
periodic_update_poses.diff (14.0 KB) - added by mmlr 12 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 12 years ago by mmlr

I have attached a patch that should fix the design of the volume space bars:

  • Tracker class does not use a BVolumeRoster anymore to iterate over all volumes
  • Only one notice is sent on pulse instead of one for each volume
  • Reworked PoseList::FindVolumePose() to FindNextVolumePose() which can be used to iterate over all volume poses
  • The PoseView now uses FindNextVolumePose() to iterate over all poses once instead of individually searching for each volume
  • The update mechanism on switching volume space bars is now unified with the one for updating the percentage (and therefore doesn't use a BVolumeRoster either)
  • Poses do now create their BVolume once and cache it for recalculation instead of always recreating them every second

May I say that the previous design was pretty much broken?

Changed 12 years ago by mmlr

Attachment: volumebar.diff added

Fixed patch that does not leak a BVolume and doesn't crash on volumes with no space bar

comment:2 Changed 12 years ago by mmu_man

Thanks, I looked into it so long ago I didn't feel like fixing. I suppose Axel is the one to apply it anyway.

Now, Couldn't we also just add volume stats to node monitoring ? This would suppress any polling, which is to be avoided whenever possible. Actually... this wouldn't work for things like NFS which don't propagate server state changes back, so it would need some polling anyway. Well, maybe that could be fixed in another way. It's also good for lowering cpu use anyway (= more battery life :)

comment:3 Changed 12 years ago by mmlr

I have now explored a different approach to fix this issue:

In the next patch I've added a global list where poses that need to be updated periodically can be registered. Currently this is only used for volume poses that need to update their volume space bar, but the mechanism is actually quite generic. It could be used for any similar periodic update. The mechanism uses a callback and a cookie, which makes it generic. This way the BVolume that is created doesn't even need to be stored in the BPose itself but is handed over as a cookie, which means not to waste unnecessary bytes on poses that are no volumes (-> like pretty much any pose except the volume poses on the desktop). Pulse uses the list to update only the volume poses and does not need to send notifications to any BPoseView anymore. Therefore the watching of setting changes has been moved from the BPoseView to the TTracker class. Performance of this setup should be linear in respect to how many volumes are mounted. It doesn't depend on the amount of open windows (BPoseViews) anymore and also saves unnecessary message passing to all windows.

Adding volume stats to node monitoring would probably be the ideal solution that would remove any polling. But as this is currently not implemented, I propose to accept this patch for now.

Changed 12 years ago by mmlr

Attachment: periodic_update_poses.diff added

comment:4 Changed 12 years ago by mmlr

Resolution: fixed
Status: newclosed

Should be fixed in hrev21462. Can you please verify?

Note: See TracTickets for help on using tickets.