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.

Sign up for GitHub

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

Jump to bottom

Drop redundant CpuBoundWork usages in lib/remote #9994

Merged
merged 3 commits into from
Feb 13, 2024
Merged

Drop redundant CpuBoundWork usages in lib/remote #9994

merged 3 commits into from
Feb 13, 2024

Conversation

yhabteab
Copy link
Member

@yhabteab yhabteab commented Feb 8, 2024 •
edited
Loading

These scopes do not need their own CPU slot to continue, but they just slow down the overall request handling and waste expensive CPU slots that would have been used by other connections awaiting to obtain a free slot. #9988 (comment)

@yhabteab yhabteab added area/distributed Distributed monitoring (master, satellites, clients) core/quality Improve code, libraries, algorithms, inline docs consider backporting Should be considered for inclusion in a bugfix release labels Feb 8, 2024
@cla-bot cla-bot bot added the cla/signed label Feb 8, 2024
julianbrost
julianbrost previously approved these changes Feb 9, 2024
View reviewed changes
Al2Klimov
Al2Klimov previously approved these changes Feb 9, 2024
View reviewed changes
@yhabteab yhabteab dismissed stale reviews from Al2Klimov and julianbrost via eb813cf February 9, 2024 14:17
@yhabteab yhabteab requested a review from julianbrost February 9, 2024 14:27
julianbrost
julianbrost approved these changes Feb 13, 2024
View reviewed changes
Copy link
Contributor

@julianbrost julianbrost left a comment

Choose a reason for hiding this comment

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

The change in lib/remote/jsonrpcconnection.cpp simply combines two separate acquisitions of CpuBoundWork into a single one.

The others are all for operations that shouldn't take long enough that it would even be worth yielding to a different coroutine.

@julianbrost julianbrost merged commit fc6a106 into master Feb 13, 2024
25 checks passed
@julianbrost julianbrost deleted the redundant-cpu-bound-work-usages branch February 13, 2024 13:53
@julianbrost julianbrost added this to the 2.15.0 milestone Feb 20, 2024
@Al2Klimov Al2Klimov mentioned this pull request Jun 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Al2Klimov Al2Klimov Al2Klimov approved these changes

@julianbrost julianbrost julianbrost approved these changes

Assignees
No one assigned
Labels
area/distributed Distributed monitoring (master, satellites, clients) cla/signed consider backporting Should be considered for inclusion in a bugfix release core/quality Improve code, libraries, algorithms, inline docs
Projects
None yet
Milestone
2.15.0
Development

Successfully merging this pull request may close these issues.
3 participants
@yhabteab @julianbrost @Al2Klimov