Simplify ShapeDescriptorIndex

[MikeAlhayek]

@sebastienros I think this can slightly help since you are working on #15620

This PR provide a slightly better improvement in the time it takes to create these object and the allocated memory. Here is a benchmark.

[sebastienros]

I think this change doesn't matter at all.

[MikeAlhayek]

I think this change doesn't matter at all.

@sebastienros are you talking about ToList() to ToArray() original change? It would just save very minor memory for each tenant.

But in the latest commit, reduces the iteration during each tenant's first request.

[Piedone]

The benchmark code look useful as well, please re-add it.

[MikeAlhayek]

@sebastienros requested to remove it :)

[Piedone]

Sebastien is wrong :D. But really, that version of the code that works with the current service (obviously you can't have comparison benchmarks between the different versions since we won't keep those) is useful both for future benchmarking, and as a sample code for benchmarks.

[sebastienros]

I highly doubt anyone will run this benchmark again. Data is known now. It's not one that ensures we don't regress some related component but one that compares two implementations. What I know though is that each and every build of OC would now have to build it too. If people really care they will look at the history of the code, find this PR and do another benchmark.

[Piedone]

Why do we have the other benchmarks in OrchardCore.Benchmarks, then?

I'd put a premium on developer productivity, what having such benchmarks with best practices in the code base provides, rather than the perhaps immeasurable impact of this class having on build time (which you only pay once without changing it anyway). It shows that we benchmarked this class, and any future changes, which will definitely come with .NET upgrades, can be benchmarked again. Nobody will look into PR histories (since with squash merging it won't even be in the repo history) for inspiration.

[MikeAlhayek]

@sebastienros I think what @Piedone is saying that way this benchmark was written can be a good example to keep in the benchmark as a code reference. None of the benchmarks show how we can setup a shell and run benchmarks against that shell. The value is in keeping the code sample not the benchmark result. Is that what you are suggesting here Zoltan?

[Piedone]

Yes, but as mentioned, apart from a sample, it'll also be useful for future benchmarking as-is.

[Piedone]

Why did you merge this, Mike? It wasn't approved yet.

[MikeAlhayek]

Spoke to @sebastienros and he is good with the benchmark being there. Also, he needs it as he is working on #15651

[Piedone]

That's great, but since I've been reviewing this PR, I'd have appreciated not merging before my approval. Not for bureaucracy, but for reviewing code changes since then.

[MikeAlhayek]

I understand. Sorry I thought you were good with it since the last thing you had was to bring back that BM. Let me know if you need changes, I can submit a follow up PR