You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by David Medinets <da...@gmail.com> on 2012/09/08 14:40:14 UTC

FindBugs: INTEGER_MULTIPLY_CAST_TO_LONG question

accumulo.core.file.blockfile.cache.LruBlockCache, line 656

  public static long calculateOverhead(long maxSize, long blockSize,
int concurrency) {
    return CACHE_FIXED_OVERHEAD + ClassSize.CONCURRENT_HASHMAP +
((int) Math.ceil(maxSize * 1.2 / blockSize) *
ClassSize.CONCURRENT_HASHMAP_ENTRY)
        + (concurrency * ClassSize.CONCURRENT_HASHMAP_SEGMENT);
  }

This code is flagged as:

 ICAST_INTEGER_MULTIPLY_CAST_TO_LONG: Result of integer multiplication
cast to long

This code performs integer multiply and then converts the result to a
long, as in:

    long convertDaysToMilliseconds(int days) { return 1000*3600*24*days; }

If the multiplication is done using long arithmetic, you can avoid the
possibility that the result will overflow. For example, you could fix
the above code to:

    long convertDaysToMilliseconds(int days) { return 1000L*3600*24*days; }

or

    static final long MILLISECONDS_PER_DAY = 24L*3600*1000;
    long convertDaysToMilliseconds(int days) { return days *
MILLISECONDS_PER_DAY; }

----

1. Can more of the value involved (say, ClassSize.CONCURRENT_HASHMAP) be longs?
2. How would this method be changed to resolve this message?
3. Can the multiplication steps be assigned to reasonably named
variables to self-document what is being added.
4. Can a comment be added why 1.2 is being used?
5. The overhead of what is being calculated?

Re: FindBugs: INTEGER_MULTIPLY_CAST_TO_LONG question

Posted by Keith Turner <ke...@deenlo.com>.
We copied this block cache code from HBase a while ago.  Need to take
a look at the changes in HBase and see if we should update our code.

On Sat, Sep 8, 2012 at 8:40 AM, David Medinets <da...@gmail.com> wrote:
> accumulo.core.file.blockfile.cache.LruBlockCache, line 656
>
>   public static long calculateOverhead(long maxSize, long blockSize,
> int concurrency) {
>     return CACHE_FIXED_OVERHEAD + ClassSize.CONCURRENT_HASHMAP +
> ((int) Math.ceil(maxSize * 1.2 / blockSize) *
> ClassSize.CONCURRENT_HASHMAP_ENTRY)
>         + (concurrency * ClassSize.CONCURRENT_HASHMAP_SEGMENT);
>   }
>
> This code is flagged as:
>
>  ICAST_INTEGER_MULTIPLY_CAST_TO_LONG: Result of integer multiplication
> cast to long
>
> This code performs integer multiply and then converts the result to a
> long, as in:
>
>     long convertDaysToMilliseconds(int days) { return 1000*3600*24*days; }
>
> If the multiplication is done using long arithmetic, you can avoid the
> possibility that the result will overflow. For example, you could fix
> the above code to:
>
>     long convertDaysToMilliseconds(int days) { return 1000L*3600*24*days; }
>
> or
>
>     static final long MILLISECONDS_PER_DAY = 24L*3600*1000;
>     long convertDaysToMilliseconds(int days) { return days *
> MILLISECONDS_PER_DAY; }
>
> ----
>
> 1. Can more of the value involved (say, ClassSize.CONCURRENT_HASHMAP) be longs?
> 2. How would this method be changed to resolve this message?
> 3. Can the multiplication steps be assigned to reasonably named
> variables to self-document what is being added.
> 4. Can a comment be added why 1.2 is being used?
> 5. The overhead of what is being calculated?