Experiment with Qt HTTP caching options #1012

HifiExperiments · 2021-02-09T21:34:35Z

potentially closes #92 and #990
fixes #1062

should theoretically

remove the need for cachebusting
improve loading times

also adds a reload button next to model URL in create

Test plan/open questions:

confirm that cachebusting scripts/models is no longer necessary (links won't update automatically, you'll still need to refresh the script or modelURL).
verify that the reload button next to model URL in create works
compare loading times (initial/loading from cache) between this PR and master. is this PR faster?
does it respect HTTP expiry times? can someone smarter than me make a test for this

HifiExperiments · 2021-02-09T21:37:15Z

libraries/networking/src/AssetClient.cpp

@@ -57,34 +55,6 @@ AssetClient::AssetClient() {
            this, &AssetClient::handleNodeClientConnectionReset);
 }

-void AssetClient::initCaching() {


more info below, but for some reason, it seems like we were only setting up caching for this AssetClient, not for all other requests? this code moved to NetworkAccessManager::getInstance()

HifiExperiments · 2021-02-09T21:39:45Z

libraries/networking/src/HTTPResourceRequest.cpp

@@ -58,7 +58,7 @@ void HTTPResourceRequest::doSend() {
    networkRequest.setHeader(QNetworkRequest::UserAgentHeader, NetworkingConstants::VIRCADIA_USER_AGENT);

    if (_cacheEnabled) {
-        networkRequest.setAttribute(QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::PreferCache);
+        networkRequest.setAttribute(QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::PreferNetwork);


this should theoretically fix the cachebusting issue, see the docs for more info: https://doc.qt.io/qt-5/qnetworkrequest.html#CacheLoadControl-enum

HifiExperiments · 2021-02-09T21:41:11Z

libraries/networking/src/NetworkAccessManager.cpp

+            QNetworkDiskCache* cache = new QNetworkDiskCache();
+            cache->setMaximumCacheSize(MAXIMUM_CACHE_SIZE);
+            cache->setCacheDirectory(cacheDir);
+            networkAccessManager->setCache(cache);


according to the docs: https://doc.qt.io/qt-5/qnetworkaccessmanager.html#setCache

"QNetworkAccessManager by default does not have a set cache."

so...I think we were forcing PreferCache above, but then...not actually setting up a cache? hopefully this new code is more correct and more performant

vircadia-build-notifier · 2021-02-09T23:25:46Z

The following links are available:
build (ubuntu-18.04, full)

build (macOS-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1012/Vircadia-PR1012-2c4fd58_b8b9cdc5.dmg

build (ubuntu-18.04, android)

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1012/Vircadia-PR1012-2c4fd58_b8b9cdc5.exe

JulianGro · 2021-02-11T07:45:01Z

So originally when we discussed cachebusting the idea was turned down because of concerns about web servers being set up properly to support that feature.
While I was always for cachebusting, has the discussion happened again and the result changed this time?

digisomni · 2021-02-11T07:50:26Z

This is implementing I think exactly what we were talking about. Webservers do have cache information in them, but currently we do not respect that. This gets us a bit closer to respecting that where possible.

HifiExperiments · 2021-02-11T07:52:43Z

yeah I think this is a pretty standard feature? this PR basically amounts to “use more default Qt values, and use them more correctly”

vircadia-build-notifier · 2021-02-22T09:19:01Z

The following links are available:
build (macOS-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1012/Vircadia-PR1012-3879882_b9b801f8.dmg

build (ubuntu-18.04, full)

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1012/Vircadia-PR1012-3879882_b9b801f8.exe

build (ubuntu-18.04, android)

Aitolda · 2021-02-24T04:25:59Z

Seems to work. A refresh button would definitely be useful. I uploaded a model,then modified is slightly in blender, reuploaded to my S3, then removed the link and repasted and the model. The updated model appeared without cache-busting.

digisomni · 2021-02-24T04:31:37Z

One way to do this would be to have the refresh button empty the field and then paste back in the URL quickly (hacky method). The other method might be to have an actual scripting API call that allows you to just trigger a reload for a resource. Given that the system handles that pretty well...

It's possible that multiple parts of the system are looking at the URL for a change, but I'm hoping that it's simply the case that the resource being refeteched would update all layers. As for falling out of sync with others, it can send a message for others' Create app's to do the same call on that model?

HifiExperiments · 2021-02-24T04:49:15Z

yeah for now the best way to do this would be to set the URL to "" and then set it back to the actual URL, just to make sure everything picks up on the change. we might just need to copy how it works for the script field? someone who understands create more than me might need to take that over

JulianGro · 2021-02-24T11:33:50Z

Theoretically you could lose the URL if the reload button removes the URL and adds it back. On Linux this happens often enough.

ctrlaltdavid · 2021-02-25T19:46:30Z

Note: In the Create app, the client script and server entity script fields both have a "reload" button.

HifiExperiments · 2021-03-21T06:02:42Z

ok added a reload button, ready for testing

vircadia-build-notifier · 2021-03-21T07:42:45Z

The following links are available:
build (ubuntu-18.04, full)

build (macOS-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1012/Vircadia-PR1012-fee508d-_2fed5124.dmg

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1012/Vircadia-PR1012-fee508d-_2fed5124.exe

build (ubuntu-18.04, android)

Aitolda · 2021-03-21T18:43:36Z

Refresh seems to work, but its worth noting the the new object will retain the dimensions or the original (reset dimensions will correct this). This is not a bug, and is desirable in many cases, but worth considering in the future.

digisomni · 2021-03-28T01:31:32Z

Compared loading The Hub content on this PR versus 2021.1.0. There doesn't seem to be any noticeable difference for me,

Is there any way to make downloads work even if links redirect? I'm not sure how the system works or how things like that work in general.

However, between caching improvements and redirecting users to downloads that are on edge servers I think we can get fast loading times.

JulianGro · 2021-03-28T02:08:02Z

Compared loading The Hub content on this PR versus 2021.1.0. There doesn't seem to be any noticeable difference for me,

My understanding is that, load times should be same or worse. Old behaviour is to just never update the cache. So whatever you downloaded would not change even if the source file changed. My understanding is that this PR contacts the server to find out if the cache is outdated.

vircadia-build-notifier · 2021-04-08T17:56:59Z

The following links are available:
build (macOS-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1012/Vircadia-PR1012-d6b3159-_2fed5124.dmg

build (ubuntu-18.04, full)

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1012/Vircadia-PR1012-d6b3159-_2fed5124.exe

build (ubuntu-18.04, android)

digisomni · 2021-04-08T21:09:01Z

Test with webserver caching options set to no cache...

digisomni · 2021-04-29T23:15:45Z

So, I've found that the refresh button doesn't update for file:/// or https:// cases, did not try others but basically I see the model blink as if it's reloading except the new model doesn't show up.

I test this by...

renaming my model on the server after loading it once
hitting refresh in the Create app, but the model stays the same it was even though it should be a whole new model, just with the exact same filename and path

However, the test for cachebusting in general worked successfully...

rename the model on the server after loading it once
go to the tutorial world under "Help" -> "Tutorial" then return back to the world
notice that the model is now the new model

I do not believe my caching options on the webserver affected either case much.

HifiExperiments · 2021-04-30T00:51:30Z

hmmm how is that different than what @Aitolda tried originally and reported success for?

Aitolda · 2021-05-15T17:28:02Z

So I finally got to digging into this again and now I'm simply confused. I tried the newest version of this PR and couldn't get it working, so I started backtracking all the way to the first release on Feb 9, which was on the old protocol and it worked suddenly. Got excited, tried again, and it didn't work. Frustrated, I tried again and this time it worked. Seems to be inconsistent.

Here's what I've been doing.

I have two models that are both simply named "cube.glb" One of these is rotated off axis.
I load up one, confirm it's working. Go to the model box in the properties dialogue and remove the link or add some gibberish.
SOMETIMES the box becomes a green outline as expected. sometimes there is no effect on screen and the model remains.

Next I replace the original file on my S3 with the alternate cube.glb.
In the properties dialogue I re-paste the link (identical). If the model had turned into the green outline previously, I now see the new model as expected, but if it remained visible, nothing happens.

Again so far I've only got this working on the original Feb 9th version but it's worth trying the others again. Will keep investigating.

daleglass · 2021-07-22T21:29:48Z

Going by Aitolda's comments it seems this needs more testing still

vircadia-build-notifier · 2021-07-31T20:18:49Z

The following links are available:
build (macOS-latest, client)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1012/Vircadia-Interface-PR1012-a2125fb-_8e472fc4.dmg

build (macOS-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1012/Vircadia-PR1012-a2125fb-_8e472fc4.dmg

build (ubuntu-18.04, full)

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1012/Vircadia-PR1012-a2125fb-_8e472fc4.exe

stale · 2022-06-18T01:47:20Z

Hello! Is this still an issue?

experimenting with qt caching

b8b9cdc

HifiExperiments added enhancement New feature or request needs testing (QA) The PR is ready for testing needs CR (code review) labels Feb 9, 2021

HifiExperiments linked an issue Feb 9, 2021 that may be closed by this pull request

Script caching isn't working properly #990

Open

digisomni added this to In progress in 2021.1.1 Eos Release via automation Feb 9, 2021

digisomni added this to the 2021.1.1 Release milestone Feb 9, 2021

HifiExperiments commented Feb 9, 2021

View reviewed changes

Merge branch 'master' into cache

b9b801f

digisomni requested a review from daleglass March 11, 2021 22:56

digisomni removed the needs testing (QA) The PR is ready for testing label Mar 11, 2021

HifiExperiments added 3 commits March 19, 2021 22:45

Merge remote-tracking branch 'upstream/master' into cache

2723c9c

fix model url change leaving behind phantom

4c1cb82

add reload button to create next to model URL

2fed512

HifiExperiments added needs testing (QA) The PR is ready for testing scripting api change Change is made in the scripting API labels Mar 21, 2021

2021.1.1 Eos Release automation moved this from In progress to Reviewer approved Mar 25, 2021

digisomni added rebuild rebuild through the GithubActions and removed rebuild rebuild through the GithubActions labels Apr 1, 2021

daleglass mentioned this pull request Apr 10, 2021

Cache busting for scripts (and other assets) needs to be revisited. #92

Closed

digisomni removed this from In progress in 2021.1.2 Eos Release May 28, 2021

digisomni added this to In progress in 2022.1.0 Selene Release via automation May 28, 2021

digisomni modified the milestones: 2021.1.2 Eos Release, 2021.2.0 Selene Release May 28, 2021

Merge branch 'master' into cache

8e472fc

daleglass approved these changes Jul 31, 2021

View reviewed changes

2022.1.0 Selene Release automation moved this from In progress to Reviewer approved Jul 31, 2021

digisomni modified the milestones: 2021.2.0 Selene Release, 2021.2.1 Selene Release Oct 30, 2021

digisomni removed this from Reviewer approved in 2022.1.0 Selene Release Oct 30, 2021

digisomni added this to In progress in 2022.1.1 Selene Release via automation Oct 30, 2021

digisomni added the dormant This PR is on hold but has interest/use surrounding it. label Nov 6, 2021

digisomni added do not merge do not merge due to issues or pending updates and removed needs testing (QA) The PR is ready for testing labels Dec 18, 2021

stale bot added the stale Issue / PR has not had activity label Jun 18, 2022

gydence mentioned this pull request Sep 21, 2023

Add cachebusting based on HTTP headers overte-org/overte#638

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Experiment with Qt HTTP caching options #1012

Experiment with Qt HTTP caching options #1012

HifiExperiments commented Feb 9, 2021 •

edited

HifiExperiments Feb 9, 2021

HifiExperiments Feb 9, 2021

HifiExperiments Feb 9, 2021

vircadia-build-notifier commented Feb 9, 2021

JulianGro commented Feb 11, 2021

digisomni commented Feb 11, 2021

HifiExperiments commented Feb 11, 2021

vircadia-build-notifier commented Feb 22, 2021

Aitolda commented Feb 24, 2021

digisomni commented Feb 24, 2021

HifiExperiments commented Feb 24, 2021

JulianGro commented Feb 24, 2021

ctrlaltdavid commented Feb 25, 2021

HifiExperiments commented Mar 21, 2021

vircadia-build-notifier commented Mar 21, 2021

Aitolda commented Mar 21, 2021

digisomni commented Mar 28, 2021

JulianGro commented Mar 28, 2021

vircadia-build-notifier commented Apr 8, 2021

digisomni commented Apr 8, 2021

digisomni commented Apr 29, 2021 •

edited

HifiExperiments commented Apr 30, 2021

Aitolda commented May 15, 2021

daleglass commented Jul 22, 2021

vircadia-build-notifier commented Jul 31, 2021

stale bot commented Jun 18, 2022

Experiment with Qt HTTP caching options #1012

Are you sure you want to change the base?

Experiment with Qt HTTP caching options #1012

Conversation

HifiExperiments commented Feb 9, 2021 • edited

HifiExperiments Feb 9, 2021

Choose a reason for hiding this comment

HifiExperiments Feb 9, 2021

Choose a reason for hiding this comment

HifiExperiments Feb 9, 2021

Choose a reason for hiding this comment

vircadia-build-notifier commented Feb 9, 2021

JulianGro commented Feb 11, 2021

digisomni commented Feb 11, 2021

HifiExperiments commented Feb 11, 2021

vircadia-build-notifier commented Feb 22, 2021

Aitolda commented Feb 24, 2021

digisomni commented Feb 24, 2021

HifiExperiments commented Feb 24, 2021

JulianGro commented Feb 24, 2021

ctrlaltdavid commented Feb 25, 2021

HifiExperiments commented Mar 21, 2021

vircadia-build-notifier commented Mar 21, 2021

Aitolda commented Mar 21, 2021

digisomni commented Mar 28, 2021

JulianGro commented Mar 28, 2021

vircadia-build-notifier commented Apr 8, 2021

digisomni commented Apr 8, 2021

digisomni commented Apr 29, 2021 • edited

HifiExperiments commented Apr 30, 2021

Aitolda commented May 15, 2021

daleglass commented Jul 22, 2021

vircadia-build-notifier commented Jul 31, 2021

stale bot commented Jun 18, 2022

HifiExperiments commented Feb 9, 2021 •

edited

digisomni commented Apr 29, 2021 •

edited