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

Use ThreadFactory to auto set thread attributes #2496

Merged
merged 8 commits into from
Aug 2, 2021

Conversation

fuzhe1989
Copy link
Contributor

@fuzhe1989 fuzhe1989 commented Jul 26, 2021

part of #2499

fuzhe1989 and others added 3 commits July 26, 2021 23:33
@fuzhe1989
Copy link
Contributor Author

/run-all-tests

@fuzhe1989
Copy link
Contributor Author

/run-all-tests

@fuzhe1989 fuzhe1989 changed the title [DNM][test] Use ThreadCreator to auto set thread attributes [DNM] Use ThreadCreator to auto set thread attributes Jul 27, 2021
@fuzhe1989 fuzhe1989 requested a review from leiysky July 27, 2021 07:01
@fuzhe1989
Copy link
Contributor Author

/run-all-tests

/// 2. ThreadName
///
/// ThreadCreator should only be constructed on stack.
class ThreadCreator
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe name with ThreadFactory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@fuzhe1989 fuzhe1989 changed the title [DNM] Use ThreadCreator to auto set thread attributes [DNM] Use ThreadFactory to auto set thread attributes Jul 27, 2021
@fuzhe1989 fuzhe1989 changed the title [DNM] Use ThreadFactory to auto set thread attributes Use ThreadFactory to auto set thread attributes Jul 28, 2021
@fuzhe1989
Copy link
Contributor Author

/run-all-tests

template <typename F, typename ... Args>
ThreadPool::Job newJob(F && f, Args &&... args)
{
auto memory_tracker = current_memory_tracker;
Copy link
Contributor

Choose a reason for hiding this comment

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

Who will provide the current_memory_tracker in current thread?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently only ProcessListElement's ctor.

@fuzhe1989 fuzhe1989 requested a review from JaySon-Huang July 31, 2021 01:31
Copy link
Contributor

@JaySon-Huang JaySon-Huang left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 31, 2021
@fuzhe1989
Copy link
Contributor Author

/run-all-tests

@fuzhe1989
Copy link
Contributor Author

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 2, 2021
@ti-srebot
Copy link
Collaborator

Your auto merge job has been accepted, waiting for:

  • 2517

@ti-srebot
Copy link
Collaborator

/run-all-tests

@ti-srebot ti-srebot merged commit bbce32a into pingcap:master Aug 2, 2021
@fuzhe1989 fuzhe1989 deleted the fuzhe/thread_creator branch August 2, 2021 07:29
@fuzhe1989 fuzhe1989 added the type/enhancement The issue or PR belongs to an enhancement. label Aug 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants