Pack OrderBy ThenBy keys in LINQ

[henriquewr]

Fixes #120785

Benchmark:

BenchmarkDotNet v0.15.4, Windows 11 (10.0.26200.6899)
AMD Ryzen 7 5700X 3.40GHz, 1 CPU, 16 logical and 8 physical cores
.NET SDK 10.0.100-rc.2.25502.107
  [Host]     : .NET 10.0.0 (10.0.0-rc.2.25502.107, 10.0.25.50307), X64 RyuJIT x86-64-v3
  Job-CZWGUH : .NET 10.0.0 (10.0.0-dev, 42.42.42.42424), X64 RyuJIT x86-64-v3
  DefaultJob : .NET 10.0.0 (10.0.0-rc.2.25502.107, 10.0.25.50307), X64 RyuJIT x86-64-v3


| Method                      | Toolchain     | Mean     | Error   | StdDev  | Gen0    | Gen1    | Allocated |
|---------------------------- |-------------- |---------:|--------:|--------:|--------:|--------:|----------:|
| OrderBoolInt                | PackedOrderBy | 533.2 us | 2.22 us | 2.08 us | 11.7188 |  3.9063 | 195.89 KB |
| OrderIntThenByDescendingInt | PackedOrderBy | 575.5 us | 6.28 us | 5.57 us | 30.2734 | 12.6953 | 499.02 KB |
| OrderBoolInt                | Default       | 801.0 us | 4.88 us | 4.07 us |  9.7656 |  1.9531 | 166.51 KB |
| OrderIntThenByDescendingInt | Default       | 923.1 us | 9.54 us | 7.97 us | 30.2734 | 11.7188 | 498.93 KB |

[Orderer(BenchmarkDotNet.Order.SummaryOrderPolicy.FastestToSlowest)]
[MemoryDiagnoser(true)]
[HideColumns("Job")]
public class Benchmarks
{
    public static readonly List<int> _list = Enumerable.Range(0, 10000).Shuffle().ToList();

    [Benchmark]
    public List<int> OrderBoolInt()
    {
        return _list
            .OrderBy(x => (x & 1) == 0).ThenBy(x => x).ToList();
    }

    [Benchmark]
    public List<int> OrderIntThenByDescendingInt()
    {
        return _list
            .OrderBy(x => x.ToString().Length).ThenByDescending(x => x).ToList();
    }
}

[henriquewr]

The purpose of this field is saving the bytes used for the packing

For example: If you pack an int and a bool, only 5 bytes will be used
and the packing will use the ulong (8 bytes) type to accommodate everything

And still have 3 bytes left for some other type

[henriquewr]

This makes the toggle for the sign bit:
1U << ((sizeof(sbyte) * 8) - 1)
would be in binary:
00000000_00000000_00000000_10000000

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-linq
See info in area-owners.md if you want to be subscribed.

[huoyaoyuan]

Generally, I don't think it's a good direction for optimization. It's increasing usage of unsafe code, which can also violate other implications like endianness and alignment. It hasn't been validated with potential trimming issues, either.

In the other hand, users can easily achieve same optimization manually with something like .OrderBy(x => (long)x.Prop1 << 32) | (x.Prop2)).

[huoyaoyuan]

This has an implication about endianness. It will get incorrect result on big endian machine.

[eiriktsarpalis]

Thanks, but I don't think this is a change we would want to pursue.

[henriquewr]

Generally, I don't think it's a good direction for optimization. It's increasing usage of unsafe code, which can also violate other implications like endianness and alignment. It hasn't been validated with potential trimming issues, either.

I agree about the unsafe code, but for this optimization it seems that is no other option, I tried writing without unsafe code but I had to fight a lot against generics and type safety, it's almost impossible to write it without unsafe code unless we actualy make a specialized implementation for every type, and because it's almost 2x performance I think is justifiable the use of unsafe

In the other hand, users can easily achieve same optimization manually with something like .OrderBy(x => (long)x.Prop1 << 32) | (x.Prop2)).

As far as I know, OrderBy(x => unchecked((ulong)(((uint)x.Value1 - int.MinValue) << 32 | ((uint)x.Value2 - int.MinValue)))) (Value1 is int and Value2 is also int) is the minimum code nescessary to safely make this optimization, and to me it seems very unlikely that somebody would write it manually

...which can also violate other implications like endianness

That's true, I forgot about big-endian

...and alignment.

I could be wrong, but doesn't Unsafe.WriteUnaligned prevent this?

It hasn't been validated with potential trimming issues, either.

I'm not very familiar with this type of issue, can you explain exactly what might cause the necessary code to be accidentally trimmed?

Is there still a chance this will make it into Linq?

If so, I can add the big endian implementation