3

I am summarizing the Code. Following Code iterates almost 4000 times in a foreach loop. On my local IIS it is working but on Production Cloud Server the complete application crashes.

try
{
    cell.Value = decimal.Parse(dr[dc.ColumnName].ToString());
}
catch
{
    cell.Value = dr[dc.ColumnName];
}

My query is :- Will Using Catch block as above cause high CPU usage?

As an addition information :- Catch is hitting 4000 times

Anup
  • 9,396
  • 16
  • 74
  • 138
  • 4
    And that's why there is [decimal.TryParse](https://learn.microsoft.com/en-us/dotnet/api/system.decimal.tryparse). You may have to do null checking separately or else .ToString() might crash it. – Peter B Jan 11 '19 at 09:42
  • There is no performance overhead adding a try catch in code. There is when exceptions are caught. They should be used to catch exceptional situations and not as a flow control. Doing so will make you fail lots of code reviews on your commits, will incur performance penalties. and also draw ire from future developers – TheGeneral Jan 11 '19 at 09:42
  • If you assign the object to an object variable, why you need to parse it at all? You are unboxing an object, parsing that to decimal and then convert the result to a string, just to finally assign this string to an object variable. Does that make sense? – Tim Schmelter Jan 11 '19 at 09:47
  • This is impossible to answer. Throwing and catching exceptions isn't free, but it's not super-expensive either. However, if you call this code in a loop, like millions of times each second, and most of the time it throws an exception, then it will spend more CPU time on this. But this is rather besides the point as there are already good examples on how to avoid exceptions altogether here, at least for the most part, by using `decimal.TryParse` instead. – Lasse V. Karlsen Jan 11 '19 at 09:48
  • Also, why do you need `.ToString()`? Which type is the underlying value to begin with? – Lasse V. Karlsen Jan 11 '19 at 09:48
  • 4
    *Throwing* is expensive, not catching. Just use `TryParse` – Panagiotis Kanavos Jan 11 '19 at 09:49
  • @TheGeneral it's the opposite. There's no overhead in *catching* an exception. The performance hit comes when the exception is thrown. – Panagiotis Kanavos Jan 11 '19 at 09:49
  • @PanagiotisKanavos Yes, i saw that, and i fully agree, however my irreverence and point remains the same :) – TheGeneral Jan 11 '19 at 09:50
  • Apart from this you shouldn´t handle exceptions which you can **avoid** in the first place. Exceptions are for something - well - exceptional. Anyway I doubt the overhead of an exception outperforms the time used for the actual db-query. Seems like your actual bottleneck is hitting the Database that often. – MakePeaceGreatAgain Jan 11 '19 at 09:50
  • 1
    If you want to know where CPU time is being spent, a profiler is a far better investment of your (and everyone else's) time than trying to *guess* at the bottlenecks and their causes. – Damien_The_Unbeliever Jan 11 '19 at 09:52
  • @Anup why the call to `ToString()`? If the value is a string, use `(string)`. If not, there's no reason to format a *number* as a string just to parse it back. That pointless format-and-parse-back hurts performance a *lot* more than exceptions – Panagiotis Kanavos Jan 11 '19 at 09:52
  • 1
    @Anup finally, all ASP.NET stacks support data binding. *Instead* of retrieving values one by one bind your grid to the data table. If `cell.Value` refers to an Excel cell, use eg `Epplus` and `sheet.Cells.LoadFromDataTable()` to load the table all at once – Panagiotis Kanavos Jan 11 '19 at 09:55
  • @Anup `On my local IIS it is working but on Production Cloud Server the complete application crashes.` that means you have other bugs. 4K rows is such a small amount of data that a single core's cache could hold the values. – Panagiotis Kanavos Jan 11 '19 at 09:57
  • Please understand that i have just summarized the Code, it is not a complete Code. There is a complex code logic. – Anup Jan 11 '19 at 10:11
  • Chekc this SO question [https://stackoverflow.com/q/1308432/488699], it looks like the jit optimizations are affected by try catch – Menahem Jan 11 '19 at 10:34

1 Answers1

6

Having try-catch present in your code has a minimal, yet IMO neglectable performance cost: in the range of < 0.001 µs per 'try-that-didnt-catch' (using a .NET 4.6.1 Release build on my Core i7 x64 machine).

But if it has to catch, then it does cost quite a bit more: in the range of 12.5 µs per 'try-that-had-to-catch' (using a .NET 4.6.1 Release build on my Core i7 machine). Still you may not notice until you get to the level of 10s of thousands of caught exceptions.

All in all it is much better to avoid throwing-and-catching if you can, and you can do so here by using decimal.TryParse.

You now have to do null checking separately or else the .ToString() might still crash it.

Rewritten code:

var v = dr[dc.ColumnName];
var s = v?.ToString(); // or you might use: var s = v as string;
if (s != null && decimal.TryParse(s, out decimal d))
    cell.Value = d;
else
    cell.Value = v;
Peter B
  • 22,460
  • 5
  • 32
  • 69
  • Why keep `ToString()` ? If the value is a string, `(string)` should be enough. If not, what type is it? Perhaps it's already a decimal? – Panagiotis Kanavos Jan 11 '19 at 09:51
  • Assuming `dr` is a `DataRow`, then `dr[dc.ColumnName]` produces an **object** which `decimal.TryParse` does not accept. And if that object is in fact a string, then `.ToString()` will simply do `(string)`. But still this is something that OP might be able to improve on. – Peter B Jan 11 '19 at 09:55
  • A type check could avoid the expensive format/parse cycle and the temporary object whether the boxed value is a string or not. The actual options for this value are a string, numeric type (which can be cast to decimal) or an invalid value – Panagiotis Kanavos Jan 11 '19 at 10:05
  • It appears that your answer is not acurate, check out this question [https://stackoverflow.com/q/1308432/488699] – Menahem Jan 11 '19 at 10:35
  • I augmented my answer to include some timings. – Peter B Jan 11 '19 at 13:07
  • Removing the `try catch` block & using `TryParse` solved my performance issue. – Anup Jan 15 '19 at 05:14