Ticket #1247 (closed bug: fixed)

Opened 20 months ago

Last modified 19 months ago

Tracker multiplies calls (open,stat) per open window

Reported by: mmu_man Owned by: axeld
Priority: low Milestone: R1
Component: Applications/Tracker Version: R1 development
Cc: Blocked By:
Platform: All Blocking:

Description

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

volumebar.diff (7.8 KB) - added by mmlr 20 months 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 20 months ago.

Change History

Changed 20 months 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 20 months ago by mmlr

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

Changed 20 months 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 :)

Changed 20 months 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 20 months ago by mmlr

Changed 19 months ago by mmlr

  • status changed from new to closed
  • resolution set to fixed

Should be fixed in r21462. Can you please verify?

Note: See TracTickets for help on using tickets.