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

feat: add request caching with OkHttp #668

Merged
merged 10 commits into from
Mar 31, 2022
Merged

Conversation

skaldarnar
Copy link
Member

@skaldarnar skaldarnar commented Sep 2, 2021

Contains

I was thinking about using a different HTTP client, especially one that brings some caching functionality with it.
Motivated by the recent changes using Java's HttpClient I took a shot at OkHttp.

For a start, this uses an OkHttpClient with caching for fetching game releases (the metadata) from Jenkins and Github (the Github library we use works with OkHttp).
To actually cache files from our Jenkins I had to "tweak" the cache control headers a bit.

The local cache files are written to the launcher's cache directy (default on Linux ~/.terasologylauncher/cache). The cache size is limited to 10MB (for now).

For me this change reduced the startup time from ~8 seconds down to 3-4 seconds.

How to test

Ensure that all tests are still passing (gradlew test) and double-check that the tests still test what they are supposed to be testing.

For manual testing:

  • ensure that the cache directory is empty
  • start the launcher normally
    • check that the cache directoy is filled when fetching game releases
  • restart the launcher
    • check that (some) requests are served from cache (logged with suffix (cached))
    • (optional) compare startup times
      • without caching: 11.5s
      • with caching: 5.7s

@skaldarnar skaldarnar added Topic: Configuration Related to configuration design, settings and storage Type: Enhancement New features or noticable improvements. labels Sep 2, 2021
@jdrueckert
Copy link
Member

Launcher improvements

Copy link
Member

@jdrueckert jdrueckert left a comment

Choose a reason for hiding this comment

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

Can we add an additional test for the newly introduced request interception/caching logic?

jdrueckert
jdrueckert previously approved these changes Mar 31, 2022
@jdrueckert jdrueckert merged commit 8d81316 into master Mar 31, 2022
@jdrueckert jdrueckert deleted the feat/caching-with-okhttp branch March 31, 2022 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Topic: Configuration Related to configuration design, settings and storage Type: Enhancement New features or noticable improvements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants