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

UX / Perf: Vue photo components performance cause spinners #862

Closed
benmccann opened this issue Jan 8, 2021 · 11 comments
Closed

UX / Perf: Vue photo components performance cause spinners #862

benmccann opened this issue Jan 8, 2021 · 11 comments
Assignees
Labels
idea Feedback wanted / feature request released Available in the stable release

Comments

@benmccann
Copy link
Contributor

benmccann commented Jan 8, 2021

Thanks for all the help with reviewing my PRs and caching improvements! And I just installed the offline PWA that you made! Woohoo!

The site's feeling relatively decent speed. I'm often seeing loading indicators and I always expected this was slow fetching of files over the network, but now with all the caching I think it's just an animation that's appearing eventhough it's probably not really having to wait for anything. I have both my wifi and cell network turned off, so it must be loading the images from offline and should be instant, but I have to wait for the spinner which makes it feel slower even though I think it's not actually waiting.

Screenshot_20210108-105700

Screenshot_20210108-105641

@lastzero
Copy link
Member

lastzero commented Jan 8, 2021

Also thought about this, but not a high priority issue from my point of view. Might help users with a slow connection.

@lastzero
Copy link
Member

lastzero commented Jan 8, 2021

Guess the spinners are more of a "performance" issue on slow clients such as mobile phones. Barely see them on my iMac in Chrome.

@benmccann
Copy link
Contributor Author

I think this is unrelated to connection speed and it's not waiting on the network at all. I think this is caused by Vue / JS taking a long time to run. I would guess that it shows up more on mobile clients though since they have slower CPUs. I'm on a 5-6 year old Thinkpad. I recently ordered a new computer - but it's on backorder 😄 The spinners were running for 8 seconds on my machine - though most of that is because I had Chrome's performance tools up. It's probably more like 1-2s normally. But based off this it looks like lots of scripting occurring. I wish I knew Vue better, so I could understand more what's happening

Screenshot from 2021-01-08 12-37-49

@lastzero
Copy link
Member

lastzero commented Jan 8, 2021

If I can keep my eyes open, I'll improve it for you... actually wanted to take a look at scroll performance right now... we'll never have to worry about people "stealing" our code, because no one else will be able to maintain it with all these optimizations 🤣

@benmccann benmccann changed the title UX: unnecessary loading spinner UX / Perf: Vue photo components performance cause spinners Jan 8, 2021
@benmccann
Copy link
Contributor Author

Thanks! That'd be great. It does seem like this and #500 could be quite related

@lastzero
Copy link
Member

lastzero commented Jan 9, 2021

Removed in master and develop for testing.

@lastzero lastzero self-assigned this Jan 9, 2021
@lastzero lastzero added idea Feedback wanted / feature request please-test Ready for acceptance test labels Jan 9, 2021
@lastzero
Copy link
Member

lastzero commented Jan 9, 2021

Found another, hidden loading transition that I could disable. In addition, UI state is now managed via CSS. Old school, but many times faster:

clipboard

Can't push this yet as it's just a proof-of-concept. Needs more testing. The other views have to be refactored as well.

@benmccann
Copy link
Contributor Author

Wow!! Amazing! Thanks once again!

I will be very curious to learn what resulted in the speed improvement since I don't know Vue as well as I know vanilla JS. Was there JS that was resizing the elements? Or was it the fact that there was one Vue component for each picture?

@lastzero
Copy link
Member

Pushed my changes, only tested on Chrome so far. Let us know if this works for you! 🐳

@benmccann
Copy link
Contributor Author

That's massively faster! Almost 20x faster by my calculations actually 🚀 🎉 Thank you x100!!!

@lastzero
Copy link
Member

Note that's we've also added gzip support. To enable it, set PHOTOPRISM_HTTP_COMPRESSION to gzip. Other algorithms like Brotli may be added later.

ZIP files and JPEGs won't be compressed again for the obvious reason. JSON responses may be up to 10x smaller.

@graciousgrey graciousgrey added released Available in the stable release and removed please-test Ready for acceptance test labels Feb 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea Feedback wanted / feature request released Available in the stable release
Projects
Status: Released 🌈
Development

No branches or pull requests

3 participants