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

Simplify ArrayTagSet sort method #1079

Closed
wants to merge 1 commit into from
Closed

Conversation

kilink
Copy link
Member

@kilink kilink commented Sep 15, 2023

Get rid of unnecessary branches in ArrayTagSet's insertionSort method.

Get rid of unnecessary branches in ArrayTagSet's insertionSort method.
@brharrington brharrington added this to the 1.6.11 milestone Sep 15, 2023
@brharrington
Copy link
Contributor

IIRC the branch was to avoid the loops in the simple case. We would need to do some testing to verify the performance with this change is better or neutral.

@brharrington brharrington modified the milestones: 1.6.11, 1.6.12 Oct 9, 2023
brharrington added a commit to brharrington/spectator that referenced this pull request Oct 25, 2023
Verify performance behavior of Netflix#1079. Case with two tags is
quite a bit slower with the simplification.
@brharrington
Copy link
Contributor

In my testing it makes a common case of two tags about 20% slower (#1088).

brharrington added a commit that referenced this pull request Oct 25, 2023
Verify performance behavior of #1079. Case with two tags is
quite a bit slower with the simplification.
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