All I have to say is :-()

Operator priorities....

But if you'd use temp variables for intermediate calculations you'd not have such a problem.

Why not just use Math.Ceiling?

I might make a suggestion to simplify the math. While Math.Ceiling() would technically work, it requires floating point math, which is obviously not preferable here.

Alternatively, if you do the following, it should give the same results with only one divide instead of two:

const int DivideAdjustment = 4*Constants.Size.Kilobyte - 1;
var transactionSizeInKb = (current->CompressedSize + sizeof(TransactionHeader) + DivideAdjustment) / (4 * Constants.Size.Kilobyte);

If current->CompessedSize + sizeof(TransactionHeader) happens to land on a 4k size, then the adjusted number will be 1 short of the next 4kB boundary so the divided value will be the same. For any value between 0 and (4kB - 1), it will push the adjusted number above the 4k boundary and the divided value will be adjusted up one. This would save one integer divide due to the mod on the second line, as well as possibly the branching penalty (depending on if that ternary gets rendered in asm as a branch or a math sign instruction).

Oren, I was curious about your statement, because I had not heard that the C# compilers and .NET JIT compilers were smart enough to do that. To test, I just did a release compile of the following code in Visual Studio 2015:

static Tuple<int, int> DivTest(int num, int div)
{
  var d = num / div;
  var m = num % div;
  return Tuple.Create(d, m);
}

On my machine, it rendered two idiv instructions in the final x64 asm. It may be that Visual Studio 2017 (C# 7.0, etc.) is smarter; I will check later today when I have access to a machine with that software.

While I do agree that the idiv instruction provides both values, it does not appear that current production C# is smart enough to take advantage of that in the common case.

Unreadability kills

Would be more obvious with proper formatting. Press Ctrl-K-D. I find that doing that (or keeping formatting not totally careless) correlates with the skill of the developer actually.

@Stuart Turner: Isn't that just because your code sample uses two intermediate variables while Oren's code doesn't use any?

Oren, I did some further testing, because I realized that you are using a constant power of two here, so the compiler can make optimizations here that it wouldn't be able to make otherwise. I wrote the following code to test compiler output:

const int ConstDivisor = 4 * 1024;
static int DivTest1(int num)
{
    return num / ConstDivisor +
        (num % ConstDivisor == 0 ? 0 : 1);
}

const int DivideAdjustment = ConstDivisor - 1;
static int DivTest2(int num)
{
    return (num + DivideAdjustment) / ConstDivisor;
}

The interesting thing to me is that the compiler still did more work for DivTest1() than for DivTest2(). In DivTest1(), it calculates num / ConstDivisor, then it calculates num % ConstDivisor, does a test on it and has a je to set a 0 or a 1 to a register, which is then added to the divided value. This could create pipeline performance penalty, though this is not likely because it's rare that you would actually hit the case where current->CompressedSize + sizeof(TransactionHeader) is on a 4k boundary.

In DivTest2(), the operation is a lot simpler. The compiler adds the constant value (using an lea instruction no less!), and then does the arithmetic shift and returns the result. 6 instructions and no jump compared to 19 instructions and a jump. Again, YMMV - this is all the results on my machine, which may be completely different to results on your machine. And in modern OOO CPUs, this may make little difference as well. I would encourage you to review it though.

Oren, you must have posted while I was writing... :-) Glad that the CLR guys recognized the missed opportunity - hopefully they can fix it soon!

Pointer + header size = overflow = 0 ?

Oren,

what tool was used to generate the screenshot in https://github.com/dotnet/coreclr/issues/3439#issuecomment-260057379 ?

Thank you

@Stephen Intel VTune