4

I am doing some custom serializing, and in order to save some space, i want to serialize the decimals as int, if possible value wise. Performance is a concern, since i am dealing with a high volume of data. The current method i use is:

if ((value > Int32.MinValue) && (value < Int32.MaxValue) && ((valueAsInt = Decimal.ToInt32(value)) == value))
{
    return true;
}

Can this be improved?

anchandra
  • 1,089
  • 12
  • 23
  • You keep talking about performance, but I don't see any benchmarks or specific requirements. How do you know that this method isn't performing well enough? How much of an improvement are you expecting to see? How will you know if you've achieved it? – Aaronaught Apr 29 '10 at 18:21
  • @aaronaught I am not saying that this method is not performing well enough, but this doesn't mean that it cannot be more efficient. For the volume of data i had, when serializing and analyzing what happens, this piece of code took 4% cpu time. Anything less is an improvement. That being said, i am not an expert on the inner works of the number representation or IL execution, so because of my lack of knowledge in this area, there might be room from improvement. – anchandra Apr 29 '10 at 18:35
  • "Performance" could also be measured in how long it takes to run. If it's only taking 4% CPU, you may be able to multi-thread this to use more CPU and finish sooner. Or did I misunderstand and this code took 4% out of the total CPU usage (maybe near 100%)? – Nelson Rothermel Apr 29 '10 at 18:58
  • I am interested in CPU usage percent of the whole serialization process. – anchandra Apr 29 '10 at 19:12
  • @anchandra: This is what's called premature optimization. In all likelihood the impact of this particular method is minuscule, and the 4% savings (or whatever it is) will have almost no visible impact. I'm all for questions on performance optimization when the performance is really an issue, but "optimizing" two arithmetic comparisons isn't a very useful exercise. We're literally talking about maybe 20 CPU instructions; if you had to process 1 **billion** values, that savings translates to 1 out of 10 seconds. Less than the time it would take to *produce* the data in the first place. – Aaronaught Apr 29 '10 at 20:03
  • 1
    @Aaronaught I don't see why it is a useless exercise. It involves understanding the internals of the number representation and choosing the most efficient way possible having this in mind. For example, i could do this by converting to string and checking for a decimal point in a string. It works, but it is not the best solution. That was the reason i did not give performance details in the question. The current code is efficient enough and in production, so it is not about premature optimization, it is about curiosity and self improvement. – anchandra May 03 '10 at 18:46

6 Answers6

1

Your invalidation criteria are:

1) Is it greater than MaxValue?

2) Is it smaller than MinValue?

3) Does it contain a fractional component?

It sounds like you have them covered. My implementation would be:

public bool IsConvertibleToInt(decimal value)
{
    if(value > int.MaxValue)
       return false;

    if(value < int.MinValue)
       return false;

    if(Math.Floor(value) < value && Math.Ceiling(value) > value)
       return false;

    return true;
}
Tejs
  • 40,736
  • 10
  • 68
  • 86
1

How about this. I think it should take fewer operations (at least a fewer number of comparisons):

    return (value == (Int32)value);

Also remember, if an if statement simply returns a boolean, you can just return the comparison. That alone might make it faster (unless the compiler already optimizes for this). If you have to use the if statement, you can similarly do this:

    if (value == (Int32)value)
    {
        //Do stuff...
    return true;
    }
    else
    {
        //Do stuff...
        return false;
    }

EDIT: I realize this doesn't actually work. I was thinking the Int32 cast would just copy in the first 32 bits from the decimal, leaving behind any remaining bits (and not throw an exception), but alas, it didn't work that way (not to mention it would be wrong for all negative values).

Seth Moore
  • 3,575
  • 2
  • 23
  • 33
  • 1
    @smoore: This throws an exception if value is too large. – D'Arcy Rittich Apr 29 '10 at 15:38
  • @OrbMan: Would unsafe do the trick and force explicit casting? – Seth Moore Apr 29 '10 at 15:56
  • @smoore - I think you mean "unchecked", but no the checked vs. unchecked context doesn't affect operations on decimal. – Jeffrey L Whitledge Apr 29 '10 at 16:02
  • There is not performance improvement between value ==(int32)value and decimal.toint32. I disassembled the decimal class, and the explicit operator for int looks like this: public static explicit operator int(Decimal value) { return ToInt32(value); } – anchandra Apr 29 '10 at 17:24
  • @anchandra: The performance improvement came from only one comparison instead of three (which would def be an improvement), but it didn't work anyways, so it doesn't really matter. – Seth Moore Apr 29 '10 at 18:08
1

It depends on how many decimal places you have or really care about. If you could say that I only care about up to 3 decimal places then the largest number you can store in int32 is int.MaxValue / 1000. If you are only working with positive numbers then you can get a higher number by using uint. In any case the way to do it is to consistently reserve space for the decimal and use * 1000 to encode them and / 1000 to decode them to / from decimal.

Nate Zaugg
  • 4,202
  • 2
  • 36
  • 53
1

Do you have any negative values? I'm guessing yes since you have the MinValue check, otherwise you can skip it. You could even use unsigned int which will allow you to convert more of your double values into ints.

Edit: Also, if you have more positive numbers, you can swap the first two conditions. That way the first one is the most likely to fail, decreasing the total number of comparisons.

Nelson Rothermel
  • 9,436
  • 8
  • 62
  • 81
  • I can have any values, with any number of decimals. Precision is a big concern – anchandra Apr 29 '10 at 18:37
  • That is actually a very good point. I will investigate this aspect. Thanks – anchandra Apr 29 '10 at 18:41
  • Depending on the nature of your numbers/requirements, you could also consider using short, int and long, or even double and float (but be careful with loss of precision). The conversion would take longer since you would have to determine the optimal type, but it would save on space. If your bottleneck is space or transmission speed (e.g. slow internet connection) and not CPU, this may be viable. – Nelson Rothermel Apr 29 '10 at 18:51
0

Wouldn't you be able to just do something like:

if(Decimal.ToInt32(value) == value)
{
     return true;
}

Not an expert on .net, but I think that should be all it'd require. Also, your two comparisons operators should be 'or equal' since the min/max values are also valid.

Edit: As pointed out in the comment, this would throw an exception. You could try catching the exception and returning false, but at that point it likely would be much faster to do the min/max testing yourself.

Kitsune
  • 9,101
  • 2
  • 25
  • 24
  • 1
    @Kitsune: This throws an exception if value is too large. – D'Arcy Rittich Apr 29 '10 at 15:38
  • @Kitsune: It is a problem of performance, not of functionality. My current solution works properly, but i want to see if i can improve its performance – anchandra Apr 29 '10 at 17:26
  • @anchandra: Yeah, that's why I didn't really recommend doing that instead, since it'd likely be slower and probably not be any more readable. – Kitsune Apr 29 '10 at 19:57
0

No need for "valueAsInt =". I believe (Decimal.ToInt32(value) == value)) gets you the same result with one less assignment. Are you using valueAsInt as some sort of output parameter?

balazs
  • 520
  • 4
  • 16
  • I need it as an output param, but the question is if i can determine the int value in a more efficient way – anchandra Apr 29 '10 at 17:27