This PR improves some float operations by ~20% (and some operations on ByteTensor by up to ~45%), but in general the performance impact seems small, unless one uses a lot of non-contiguous tensors and/or broadcasting with large dimensions.

Here's an example where I could get ~20% improvement on GTX 1080:

A = torch.cuda.FloatTensor(1000, 256)
B = torch.cuda.FloatTensor(128)
A = A[:, :128]
A.pow_(B)  # Improves from ~7.6 to ~6.0 usec

I found at least one case where it becomes slower by ~5%, but such cases seem to be rare, so I still think it's a net performance win on average, although small.

A = torch.cuda.IntTensor(2048, 2048)
A = A[:, :2000]
B = torch.cuda.IntTensor(2000).fill_(10)
A.remainder_(B)  # Changes from ~180 to 189 usec.

Raw benchmark results are https://github.com/yongjik/pt_test/tree/master/results/offset in case anybody's interested.

• This PR also fixes overflow errors with large tensors (2G elements or more), which makes me suspect that nobody has actually been using 64-bit index math so far...

@pytorchbot add to whitelist

Hi @wickedfoo, thanks for the detailed review, and I understand your point that the code is too complicated for the (rather unimpressive) speedup. I'll try just using the constant division algorithm and get back to you. Might take a few days.

On the other hand, I do think there's a measurable speedup for some cases. One case I found:

A = torch.cuda.FloatTensor(4096, 4096)
B = torch.cuda.FloatTensor(2048)
A = A[:, :2048]
A.pow_(B)  # Changes from ~389 us to ~315 us (speedup ~23%)

Ironically, using even larger tensor doesn't show larger speedup, because then (I suppose) memory bandwidth dominates everything.

@yongjik also take a look at https://github.com/milakov/int_fastdiv

Also the reason that IndexType was unsigned int rather than signed int for 32-bit indices was that I discovered that div/mod was faster on unsigned rather than signed types (relevant for K40 and M40 I think?), but this may no longer be the case with more recent architectures. It might be worth comparing both signed and unsigned int32 for IndexType to see if there's still a difference.

The integer division by magic constants code in the Caffe2 source I think will be faster than int_fastdiv if you exclude the -1 / 1 case. They're basically the same code more or less, except you avoid this additional work:

https://github.com/milakov/int_fastdiv/blob/master/int_fastdiv.h#L126

@yongjik I suffered a lot tuning the indexSelectKernel and I found the kernel is mainly limited by read_throughput. I suspects that we should focus more on reducing the memory read operations and improving the locality, rather than only computational cost.

Hi @wickedfoo, I updated the code to remove the increment stuff and only leave the int division algorithm. Could you take another look?

Regarding signed/unsigned integer, I think the point is moot, because (in the references I found) the fast division algorithm for signed integers always has more operations than the unsigned version. So I think they don't really give us any benefit here.

Hi guys, any thoughts on this PR?

Hi @wickedfoo, could you give your opinion? If this PR still looks like too much complication, I understand if you don't want to merge this, but I'd appreciate a decision rather than this PR staying in limbo forever. Thanks!

@yongjik i think he does not get github notification emails. I will ping him directly. sorry for delay.

Looking now.

Hi @wickedfoo, thanks for the review.

I ran several hundred configurations of tensor operations (on GTX 1080 / CUDA 9.1), including add, mul, tanh, pow, and remainder, on tensors with sizes ranging between 1000x64 and 4096x4096. About 1/3 of them showed speedup of 5% or more. There are some cases when performance improves by up to ~50%. For the rest, performance didn't change meaningfully (within 5%). I saw several cases of slowdown of ~5%, but these operations were too small (< 20 us) and I couldn't reproduce them reliably: might be just random noise.

The biggest win I could find was:

A = torch.cuda.HalfTensor(4096, 4096)
B = torch.cuda.HalfTensor(2048)
C = torch.cuda.HalfTensor(4096, 4096)

# ~266 us to ~172 us (speedup 55%)
torch.mul(A[:, :2048], B, out=C[:, :2048])

A = torch.cuda.HalfTensor(8192, 8192)
B = torch.cuda.HalfTensor(4096)
C = torch.cuda.HalfTensor(8192, 8192)

# ~1.06 to ~0.69 ms (speedup 54%)
torch.mul(A[:, :4096], B, out=C[:, :4096])

We also have speedup for float operations, though not as dramatic:

# ~26 to ~21 us (speedup 24%, but it's too noisy so might not be accurate)
nrows, ncols, ncols2 = 1024, 1024, 512

# ~365 to 312 us (speedup 17%)
nrows, ncols, ncols2 = 4096, 4096, 2048

# ~1.38 to 1.17 ms (speedup 18%)
nrows, ncols, ncols2 = 8000, 8000, 4000

A = torch.cuda.FloatTensor(nrows, ncols)
B = torch.cuda.FloatTensor(ncols2)
A = A[:, :ncols2]

A.pow_(B)

For some other float operations, I observed speedup of ~25% for mid-size tensors (around 1000x128), but it becomes smaller as tensors get bigger (~9% for 1024x1024, ~3% for 8000x3000), probably because memory latency dominates everything for these tensors.

@yongjik thanks a lot for the PR it's good to go.
@ezyang can you check if the failing perf-regression test is legit or false-positive?

I don't know if I'm doing it right, but I followed the advice of the failed test log and ran cd .jenkins/perf_test/ && bash test_gpu_speed_mlstm.sh:

On clean branch (2726550, ran three times):

Runtime stats in seconds:
{"mean": 2.3622500000000004, "sigma": 0.01499624953113275}
{"mean": 2.37195, "sigma": 0.01663272376972571}
{"mean": 2.3788499999999995, "sigma": 0.01288128487380043}

With this PR on top of it:

{"mean": 2.3658499999999996, "sigma": 0.018205150370156273}
{"mean": 2.3644000000000007, "sigma": 0.021511392330576876}
{"mean": 2.36015, "sigma": 0.01142048597915163}

So I think there's no meaningful difference on GTX-1080, but other GPUs might report different numbers, I guess.

The GPU perf tests have been flaky recently, so you should ignore them for the purposes of assessing this PR.

thanks @yongjik. sorry for the delay in review.

No worries! Half of the delay was mine, after all. Thanks for the review.