Skip to content

Conversation

@SergeiPavlov
Copy link
Collaborator

@SergeiPavlov SergeiPavlov commented Nov 25, 2025

I see in dumps that EnumerableExtensions.ToArray() is a hotspot here

@botinko
Copy link

botinko commented Nov 25, 2025

Imo linq concat could be better here, need to benchmark.

@SergeiPavlov
Copy link
Collaborator Author

SergeiPavlov commented Nov 25, 2025

Imo linq concat could be better here, need to benchmark.

Concat() and .ToArray() allocate at least 2 iterators
There is no way to avoid it
foreach() optimization can avoid them

And we cannot benchmark every change
That will slow down dev process 10 times

@botinko
Copy link

botinko commented Nov 26, 2025

Iterators could be stack-allocated or even stripped from final code.

We achiving quality, not numbers of optimizations.

@botinko
Copy link

botinko commented Nov 26, 2025

We don't need bechmarks every change, there a lot of places where optimization is obvious. Here it's questionable.
I think AI could create a bech for you in a seconds.

Copy link

@botinko botinko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in Slack, it just slightly faster, then Columns.Columns.Concat(columns).ToArray(); (without passing n)

@SergeiPavlov SergeiPavlov merged commit f58d042 into master-servicetitan Nov 27, 2025
38 checks passed
@SergeiPavlov SergeiPavlov deleted the RecordSetHeader branch November 27, 2025 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants