View unanswered posts | View active topics



Reply to topic  [ 6 posts ] 
ThemeManager eating exceptions causing slowdown patch 
Author Message
Member

Joined: Thu Aug 23, 2012 5:49 am
Posts: 7
Reply with quote
While I was using FlashDevelop in debug mode, I noticed that my FlashDevelop was slowing down to a crawl. I looked at my Visual Studio output and FlashDevelop was eating hundreds of exceptions a minute from the same line of code.

I checked out ThemeManager and saw that to get a theme value, it was eating an exception to do so because the value it was looking for was not in valueMap. I changed the code to use a TryGetValue from the dictionary instead. This is significantly faster because eating exceptions is very expensive. I also modified GetThemeColor to use the new property.

Old culprit code
Code:
public static String GetThemeValue(String id)
{
    try { return valueMap[id]; }
    catch { return null; }
}

New patch
Code:
Index: FlashDevelop/Managers/ThemeManager.cs
===================================================================
--- FlashDevelop/Managers/ThemeManager.cs   (revision 2639)
+++ FlashDevelop/Managers/ThemeManager.cs   (working copy)
@@ -22,10 +22,12 @@
         /// <summary>
         /// Gets a value entry from the config.
         /// </summary>
-        public static String GetThemeValue(String id)
+        public static String GetThemeValue (String id)
         {
-            try { return valueMap[id]; }
-            catch { return null; }
+           string result;
+           if (valueMap.TryGetValue (id, out result))
+              return result;
+           return null;
         }
 
         /// <summary>
@@ -33,7 +35,7 @@
         /// </summary>
         public static Color GetThemeColor(String id)
         {
-            try { return ColorTranslator.FromHtml(valueMap[id]); }
+            try { return ColorTranslator.FromHtml(GetThemeValue(id)); }
             catch { return Color.Empty; }
         }



Sun Apr 07, 2013 2:10 am
Profile
Admin

Joined: Tue Aug 30, 2005 6:14 pm
Posts: 3045
Location: Finland
Reply with quote
Thanks, good catch. Added to SVN.


Sun Apr 07, 2013 6:25 pm
Profile WWW
Member

Joined: Thu Aug 23, 2012 5:49 am
Posts: 7
Reply with quote
I actually have a minor concern I thought of after I submitted the patch. It might rely on the fact that it used to eat the exception and now is exceptionless code.

When it would catch an exception before, it would return Color.Empty. Now it just returns NULL. It could cause a bug. I have made a correction to GetThemeColor that preserves the old behavior.

Code:
public static Color GetThemeColor(String id)
{
        var value = GetThemeValue (id);
        return value != null
                ? value
                : Color.Empty;
 }


Sun Apr 07, 2013 11:26 pm
Profile
Admin

Joined: Tue Aug 30, 2005 6:14 pm
Posts: 3045
Location: Finland
Reply with quote
ColorTranslator.FromHtml returns Color.Empty if the color param is null. And it throws an exception if its an invalid value. I think it's fine as it is.


Mon Apr 08, 2013 6:30 am
Profile WWW
Member

Joined: Thu Aug 23, 2012 5:49 am
Posts: 7
Reply with quote
Ah right, I hadn't realized that. Sounds good then!


Mon Apr 08, 2013 6:27 pm
Profile
Admin

Joined: Wed Aug 31, 2005 7:27 am
Posts: 12172
Location: London
Reply with quote
Wow looks like a bad idea to rely on exceptions here.
The right way is usually to do:
Code:
if (collection.containsKey(key)) return collection[key];
return defaultValue;


Sat Apr 13, 2013 12:04 pm
Profile WWW
Display posts from previous:  Sort by  
Reply to topic   [ 6 posts ] 

Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum

Search for:
cron
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group.
Designed by ST Software for PTF.