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

buildrequestdistributor: Sort pending builders only once #7512

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vibbo
Copy link
Contributor

@vibbo vibbo commented Apr 16, 2024

Sorting pending builder each time the list is updated is unnecessary overhead. This can be done once the list is passed on to BuildRequestDistributor._activityLoop.

  • I have updated the unit tests
  • I have created a file in the newsfragments directory (and read the README.txt in that directory)
  • I have updated the appropriate documentation

I was debugging some issues with BuildRequestDistributor and I find this change is an improvement of the existing implementation.
The question is, am I missing something? Is there actually any reason for the current implementation where pending builders are sorted each time the list is updated?

I will create newsfragment if the change is viable.

@vibbo vibbo force-pushed the private/vibbo/improvement/build_request_distributor branch from 61dbdfb to 01cfbab Compare April 16, 2024 15:28
@vibbo vibbo marked this pull request as ready for review April 16, 2024 15:44
Sorting pending builder each time the list is updated is unnecessary overhead.
This can be done once the list is passed on to BuildRequestDistributor._activityLoop.
@vibbo vibbo force-pushed the private/vibbo/improvement/build_request_distributor branch from 01cfbab to 4a2050d Compare April 23, 2024 14:06
@p12tic
Copy link
Member

p12tic commented Apr 28, 2024

I'm not confident with merging this change. I would need to review the algorithm used.

@vibbo
Copy link
Contributor Author

vibbo commented May 6, 2024

I'm not confident with merging this change. I would need to review the algorithm used.

Is there anything I can do to help now?

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.

None yet

2 participants