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

implemented multithreading using the multiprocessing module #1116

Open
wants to merge 10 commits into
base: 2.18-Beta
Choose a base branch
from

Conversation

virophagesp
Copy link

@virophagesp virophagesp commented May 14, 2024

Related Issue/Addition to code

Old-Multi-Threading-Attempt

  • Fixes "Multithreading works but too slow"
  • Addition to code.

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Proposed Changes

  • utilizing the multiprocessing module to multithread get_comments

Why is this change needed?

It's needed because it speeds up scanning alot

Additional Info

  • the threading module does not use multiple cpu cores, which is why the old attempt was too slow

Checklist:

  • My code follows the style guidelines of this project and I have read CONTRIBUTING.md
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

converted YOUTUBE variable to a list of 2 builds rather than a single 1
replace instances of auth.YOUTUBE with auth.YOUTUBE[0]
in the get_comments function, multithreading has been added
changed an instance of auth.YOUTUBE to auth.YOUTUBE[0]
replaced instances of auth.YOUTUBE to auth.YOUTUBE[0]
it is about the second element
@virophagesp
Copy link
Author

What documentation is my checklist referring to?

@virophagesp
Copy link
Author

this is currently a basic setup for multi-threading the scanning of comments using only 2 CPU cores
I plan to add for loops for core scaling later
The reason why is because If multiple additional threads are made, large amounts of data shared between them will cause lag, and they share alot of data
If this is merged then this will be a major problem to fix

@virophagesp
Copy link
Author

@ThioJoe I think multithreading is done, what do you think?

@virophagesp
Copy link
Author

found a bug that causes it to freeze, I will reopen when its fixed and tested for other bugs

@virophagesp
Copy link
Author

i also found out i forgot to implement shared memory which might be the cause
this might take a while

@virophagesp
Copy link
Author

i implemented shared memory but now there is way too much ram usage and a unrelated bug i am currentlly fixing

@virophagesp
Copy link
Author

i fixed the memory leak but there is still the issue of freezing on certain videos after 1 thread finishes

@virophagesp virophagesp reopened this May 25, 2024
@virophagesp
Copy link
Author

I am reopening this since it is mostly fixed
I also found the freezing issue to be uncommon

@virophagesp
Copy link
Author

I found a variety of more videos with the freezing issue
I have found it to be caused by certain comments with replies not taking in the pageToken input and causing an infinite loop
What separates them from most comments with replies that dont cause this issue is unclear
I plan to investigate more this week and hopefully fix it

@virophagesp
Copy link
Author

I found out that it is for any comment with more than 100 replies
But also it isnt caused by my changes at all

@virophagesp
Copy link
Author

@ThioJoe this is ready now

@4yman-0
Copy link

4yman-0 commented Jun 25, 2024

Can you explain why there's 1 reference to YOUTUBE[1]

@virophagesp
Copy link
Author

It is only for multi threading, I tried using it like it was before but errors were caused so unfortunately there needs to be 2 of them, maybe if other things get multithreaded it will have other uses

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