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 Process to actually run the task in multiple processes to speed up the performance #64

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

minhhieugma
Copy link
Contributor

@minhhieugma minhhieugma commented Apr 22, 2023

  • Use Process to actually run the task in multiple processors to speed up the performance
  • This PR is meant to solve "Always run on 1 core CPU only "

Before

  • The rate is about 5,000 tokens/s

After

  • The rate is about 70,000 tokens/s
  • With the same computer, 8 logical processors

Issue Link

print("\n Starting ...")
self.start_t = time()

if self.use_process:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Due to the fact that I don't have enough time to convert all tasks to using the Process. I converted the random_brute. That's why I need this flag

Copy link
Owner

Choose a reason for hiding this comment

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

It will be better if the value is declared in the if __name__ == "__main__": and then passed to the class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want use_process to be used for this particular option. The rest should be used the old approach. Then, slowly we can migrate to the new approach.

If we move use_process to the outside of the class, how can we switch and meet this expectation?

Copy link
Owner

Choose a reason for hiding this comment

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

I mean to share the values between speed() and the rest.
The problem is that we need to measure the speed and display it. Thus, the current number should be shared to the speed() function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that the metrics are shared across the threads. It means that t1 = threading.Thread(target=obj.speed) is able to display it correctly.

In details, these lines of code help to to share metrics between speed()

self.cur_n = self.manager.Value('i', 0)
self.start_t = self.manager.Value('i', 0)

Could you specify which information is not shown so that I can look deeper?

@minhhieugma
Copy link
Contributor Author

Please ignore the CodeFactor. It is an existing issue. I will try to improve it in the next coming PRs

Copy link
Owner

@vlnahp vlnahp left a comment

Choose a reason for hiding this comment

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

I'll check if it really works and merge soon.

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