Quantized Matmul: Small batches are slower than no-batch

[lucasavila00]

Thanks for the work on this repo, it's amazing.

The newly added quantized matmul kernels are great, but they're slower than using the vec kernels on small batch sizes.

On Llama.cpp I see they use the vec version if batch is <= 4 ggml-org/llama.cpp#5351 ggml-org/llama.cpp#5370

I'm reading https://github.com/huggingface/candle/blob/main/candle-kernels/src/quantized.cu and it seems the code was extracted from Llama.cpp before this PR was merged, and candle's version misses this optimization.

For context, this issue was noticed on mistral.rs EricLBuehler/mistral.rs#139

[LaurentMazare]

Interesting, do you know if the new kernels are slower than the old approach for your use case? (the old approach was to dequantize then matmul so would not involve the mmv kernels). If that's the case, switching back to the old style when the input size is smaller than 4 should be a straightforward change.

[lucasavila00]

Old kernels:

batch=1 20ms
batch=2 278ms
batch=3 354ms

New kernels:

batch=1 20ms
batch=2 133ms
batch=3 144ms

They are faster. But not batching has more throughput.

On larger batch sizes the throughput beats no-batching.

[lucasavila00]

What I think is happening is that candle is not checking this condition: https://github.com/ggerganov/llama.cpp/blob/master/ggml-cuda.cu#L1898-L1900

And then not using the vec kernels: https://github.com/ggerganov/llama.cpp/blob/master/ggml-cuda.cu#L1955

The performance issues are similar to what's described in the body of this issue ggml-org/llama.cpp#5351 (comment)

I'm not too familiar with candle's source code but I'm profiling:

bs=1

bs=2

I think around here https://github.com/huggingface/candle/blob/main/candle-core/src/quantized/cuda.rs#L387-L391 it should also use the vec kernels if BS<8 (latest llama.cpp config)

[lucasavila00]

For reference, bs=2 and llama.cpp launches the _vec kernels

[LaurentMazare]

Ok, thanks for the details, I'm testing it with #2075 the downside is that it adds a lot of kernels (even up to only batch size 4) so will have an impact on the binary size, but hopefully it's worth it.

[EricLBuehler]

Yes, we saw a great performance increase after MMVQ was added, so hopefully this will improve small batch performance.

[LaurentMazare]

#2075 has been merged, would be great if you can re-run the benchmarks and see if it improved things.

[EricLBuehler]

@lucasavila00, I've added this to our fork so it is ready to test.

[lucasavila00]

bs=1 18ms
bs=2 22ms
bs=3 33ms
bs=4 37ms
bs=5 120ms
bs=6 125ms
bs=7 130ms
bs=8 135ms
bs=9 140ms

Thank you! It improved things a lot.

Is implementing this up to 8 out of question, due to binary size?

[LaurentMazare]

Great that it worked out. I just tried out adding the kernels up to 8 in #2077 and it's a ~10% increase of binary size, from 19.6MiB to 21.4MiB on my box so I'm a bit on the fence for this - as it only affects cuda targets that might be fine though.

[lucasavila00]

Thank you!