After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 632269 - Log slow commands
Log slow commands
Status: RESOLVED FIXED
Product: hyena
Classification: Other
Component: Hyena.Data.Sqlite
git master
Other Linux
: Normal normal
: ---
Assigned To: hyena-maint
hyena-maint
Depends on:
Blocks:
 
 
Reported: 2010-10-16 00:37 UTC by David Nielsen
Modified: 2011-04-22 19:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (1.54 KB, patch)
2010-10-16 00:37 UTC, David Nielsen
accepted-commit_now Details | Review
updated patch (1.51 KB, patch)
2010-11-25 17:01 UTC, David Nielsen
none Details | Review
Once more with feeling (1.50 KB, patch)
2010-11-25 17:17 UTC, David Nielsen
none Details | Review

Description David Nielsen 2010-10-16 00:37:48 UTC
Created attachment 172472 [details] [review]
proposed patch

Right now we don't log slow running commands as errors, doing so would increase the quality of bug reports and cut the requirement to ask users for separate --debug-sql logs. It would also make SQL related performance issues easier to spot quickly
Comment 1 Raimo Radczewski 2010-11-06 03:02:09 UTC
I talked with david about this, and imho if there really is a significant amount of bugs or complains concerning banshee's performance this would probably the best way to nail the problem fast (and with the help of the users)
Comment 2 Alex Launi 2010-11-25 16:29:13 UTC
Review of attachment 172472 [details] [review]:

Looks good, just update for style

::: Hyena.Data.Sqlite/Hyena.Data.Sqlite/HyenaSqliteCommand.cs
@@ +62,3 @@
         private object [] current_values;
         private int ticks;
+        const int threshold = 500;

Fix capitalization and move to top of class

private const int THRESHOLD = 500;

private object result = null;
Comment 3 David Nielsen 2010-11-25 17:01:52 UTC
Created attachment 175249 [details] [review]
updated patch

Thank you for the review, there is the updated version with your requested changes for HACKING compliance.
Comment 4 David Nielsen 2010-11-25 17:17:26 UTC
Created attachment 175253 [details] [review]
Once more with feeling

And now it actually compiles
Comment 5 Alex Launi 2010-11-25 19:26:29 UTC
My only question is the threshold. Is 500 a good threshold for this? If it's too low we'll get slammed with bug reports saying banshee told them to file a bug, if it's too high it's ineffective.
Comment 6 David Nielsen 2010-11-26 02:38:01 UTC
I just picked a number for the sake of it. I think an good setting for this would be 100ms. Looking at what my modest dual core atom machine with a slow drive I see worst case times in debug-sql mode around 50-70ms in normal use. I think >100ms would be abnormal enough to both be felt as slow and not get triggered often in normal use.
Comment 7 David Nielsen 2010-12-03 03:34:35 UTC
Half a second is an absurd amount of time, however 100ms in experimentation on my machine seems to trigger to often for comfort. Perhaps we should put our foot down and say 250 ms. 1/4 of a second is certainly going to be felt and if we cannot fulfill that requirement on a modern machine are doing it wrong(tm)?

It really depends on how we think this can be used best. If we are wanting to urge ourselves to do better I would say 250ms aiming to lower it with every major release cycle towards 100ms. If we are merely looking for the worst cases we should set this high, say 1000ms.

I think perhaps the best way to go about this is to use a low setting for Active Development releases and a high setting for maintained releases.
Comment 8 David Nielsen 2010-12-23 18:36:33 UTC
I've started documenting all the common steps I am aware of for performance bugs. 

http://live.gnome.org/Banshee/CommonQuestions/PerformanceIssues

In the process I experimented on a couple of machines and found that 1 full second seemed sufficient to not trigger this with disturbing frequency (which sounds like a lot in regular use depending on the task).
Comment 9 Gabriel Burt 2011-04-22 19:58:08 UTC
I committed a patch that, in --debug mode, will log cmds that take over 500 ms to run.  This minimizes the risk of spewing tons of logging for lots of users, getting lots of bug reports etc.