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

Improved data retrieval in GitHubData class #48

Merged
merged 9 commits into from
Aug 16, 2018

Conversation

rroyGit
Copy link
Contributor

@rroyGit rroyGit commented Aug 16, 2018

Once GitHubThread is initiated, a new thread is started - responsible for checking if cache is valid.
Locks are used to handle the two threads, http call starts running asap after cache thread has finished. Http thread maintains cache property set by the owner - no http connection calls if cache is last modified within the last 10 days.
In case http thread runs before cache thread, http thread checks if cache thread is alive - if yes, http thread waits until cache thread successfully finished.

This should solve the loading lag if the cause was making http requests.
Let me know how it goes for you.

@fennifith
Copy link
Owner

Oh boy, I've... never actually seen anyone use a Semaphore before. This looks good. I shall test.

@fennifith
Copy link
Owner

Upon initial inspection, it appears that doHttpConnection() is never called, even when the cache isn't present, and so nothing is passed to the init method and... nothing loads. Not loading anything has definitely sped up the loading speed a lot though, so great work!

Lol. I'm currently putting in an excessive amount of log statements to see if I can figure out where it's going wrong.

@fennifith
Copy link
Owner

Found it. It is not quite as speedy now, but definitely an improvement over what was there before. Thank you for contributing!

@fennifith fennifith merged commit d78c81e into fennifith:develop Aug 16, 2018
@rroyGit
Copy link
Contributor Author

rroyGit commented Aug 16, 2018

Cool, ty

@rroyGit rroyGit deleted the develop branch August 16, 2018 02:32
@rroyGit rroyGit restored the develop branch August 16, 2018 02:32
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