Skip to content
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

fix: multiple performance fixes for type serialization feat: adds to identifier mapping to non nullable enum #1915

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

baywet
Copy link
Member

@baywet baywet commented Nov 12, 2024

Signed-off-by: Vincent Biret [email protected]

feat: adds to identifier mapping to non nullable enum

Signed-off-by: Vincent Biret <[email protected]>
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
79.0% Coverage on New Code (required ≥ 80%)
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@MaggieKimani1
Copy link
Contributor

I understand that using a bitwise operation to determine the number of flags set on JsonSchemaType is more efficient (O(1)) than using the iterative count (O(n)) as I had implemented. However the performance gain seems minimal if JsonSchemaType contains only a few flags (e.g., up to 3).

@baywet
Copy link
Member Author

baywet commented Nov 13, 2024

It's not just the loop, it's also:

  • Enum.GetValues needs to go through the type system to get the information, which is much slower\
  • Assigning count multiple times is slower than a single bitwise instruction

(besides the other improvements in terms of "caching" the enum values, avoiding auto-boxing, etc...

@baywet baywet merged commit 5fef51c into vnext Nov 13, 2024
10 of 11 checks passed
@baywet baywet deleted the fix/performance-type-serialization branch November 13, 2024 19:26
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.

2 participants