Skip to content

Avoid reflection when constructing row in bsatn deserialization #2677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

joshua-spacetime
Copy link
Collaborator

@joshua-spacetime joshua-spacetime commented Apr 29, 2025

Description of Changes

Avoids paying new T()'s (potential) cost of reflection on each row decode.

API and ABI breaking changes

None

Expected complexity level and risk

1

Testing

  • Unity profile

@joshua-spacetime joshua-spacetime changed the title Instantiate row type once and reuse in bsatn deserialization Construct row once for reuse in bsatn deserialization Apr 29, 2025
@joshua-spacetime joshua-spacetime force-pushed the joshua/perf/avoid-reflection-in-row-decoding branch from 036680e to 2a49ec8 Compare April 29, 2025 01:02
@joshua-spacetime joshua-spacetime changed the title Construct row once for reuse in bsatn deserialization Avoid reflection when constructing row in bsatn deserialization Apr 29, 2025
@joshua-spacetime joshua-spacetime changed the base branch from joshua/perf/csharp-enum-validation to master April 29, 2025 18:00
@joshua-spacetime joshua-spacetime changed the base branch from master to joshua/perf/csharp-enum-validation April 29, 2025 18:00
@joshua-spacetime joshua-spacetime force-pushed the joshua/perf/avoid-reflection-in-row-decoding branch from 2a49ec8 to c62b26a Compare April 29, 2025 22:58
Base automatically changed from joshua/perf/csharp-enum-validation to master April 29, 2025 22:59
@joshua-spacetime joshua-spacetime force-pushed the joshua/perf/avoid-reflection-in-row-decoding branch from c62b26a to 4fba3c2 Compare April 29, 2025 22:59
@joshua-spacetime joshua-spacetime marked this pull request as ready for review April 29, 2025 23:00
@kazimuth
Copy link
Contributor

I'd like to check if this actually monomorphizes in Unity

@CLAassistant
Copy link

CLAassistant commented May 3, 2025

CLA assistant check
All committers have signed the CLA.

@bfops bfops added release-any To be landed in any release window performance A PR/Issue related to improving performance of stdb labels May 5, 2025
@kazimuth
Copy link
Contributor

kazimuth commented May 5, 2025

From deep profiling Blackholio, this does not seem to result in monomorphization.

I found https://andrewlock.net/benchmarking-4-reflection-methods-for-calling-a-constructor-in-dotnet/ which shows that activator.createinstance is still quite fast -- 40ns, versus 10ns for new(). I think this may be a case of the unity deep profiler blowing up execution times. But I'm not completely confident about that.

@joshua-spacetime joshua-spacetime force-pushed the joshua/perf/avoid-reflection-in-row-decoding branch from 4fba3c2 to 85be484 Compare May 8, 2025 21:53
@joshua-spacetime
Copy link
Collaborator Author

A bot test showed no improvement. Closing this in favor of the following codegen approach #2725.

@joshua-spacetime joshua-spacetime deleted the joshua/perf/avoid-reflection-in-row-decoding branch May 9, 2025 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance A PR/Issue related to improving performance of stdb release-any To be landed in any release window
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants