files-list: performance optimizations #40958
Conversation
| import NcIconSvgWrapper from '@nextcloud/vue/dist/Components/NcIconSvgWrapper.js' | ||
| import NcLoadingIcon from '@nextcloud/vue/dist/Components/NcLoadingIcon.js' | ||
| import CustomElementRender from '../CustomElementRender.vue' |
| class="files-list__row-icon-favorite" | ||
| :aria-label="t('files', 'Favorite')"> | ||
| <FavoriteIcon /> | ||
| <FavoriteIcon v-once /> |
There was a problem hiding this comment.
Icons don't need re-rendering
| // Table head, body and footer | ||
| tbody { | ||
| will-change: scroll-position, padding; | ||
| contain: layout paint style; |
There was a problem hiding this comment.
Some CSS tricks to improve rendering performance
| // Adding scroll listener AFTER the initial scroll to index | ||
| this.$el.addEventListener('scroll', this.onScroll) | ||
| this.$el.addEventListener('scroll', this.onScroll, { passive: true }) |
There was a problem hiding this comment.
Passive listeners are faster
| // Max 0 to prevent negative index | ||
| this.index = Math.max(0, index) | ||
| this.$emit('scroll') | ||
| this._onScrollHandle ??= requestAnimationFrame(() => { |
There was a problem hiding this comment.
Throttling this call to once per frame. Notably, this.$el.scrollTop is a getter and causes layout thrashing, so this is the most important change here.
There was a problem hiding this comment.
That's something I wanted to experiment with! Nice take at this issue! It make sense :)
| &::v-deep { | ||
| // Table head, body and footer | ||
| tbody { | ||
| will-change: scroll-position, padding; |
There was a problem hiding this comment.
Not sure about padding. Is it even a valid value for will-change ? 🤔
There was a problem hiding this comment.
Well will-change takes <custom-ident> as a valid value so it's all up to the browser. I don't think there are any real optimizations for padding changes right now though 🤷🏻
skjnldsv
force-pushed
the
pulsejet/files-list-perf
branch
from
6a6fde6 to
fdca64b
Compare
|
Pushed some fixes, performances are definitely positively impacted!! 🚀 |
skjnldsv
added
3. to review
skjnldsv
requested review from
a team,
artonge,
nfebe and
sorbaugh
and removed request for
a team
|
Thanks for fixing this up @skjnldsv 😄 |
Signed-off-by: Varun Patil <[email protected]>
skjnldsv
force-pushed
the
pulsejet/files-list-perf
branch
from
fdca64b to
08c641d
Compare
|
/compile amend / |
Signed-off-by: John Molakvoæ <[email protected]> Signed-off-by: nextcloud-command <[email protected]>
nextcloud-command
force-pushed
the
pulsejet/files-list-perf
branch
from
08c641d to
4db0388
Compare
|
Drone failure unrelated: https://drone.nextcloud.com/nextcloud/server/42176 |
|
This seems to have caused a regression where file list headers are broken now and not rendering anymore. Namely recommendations app and text app readme rendering |
Some optimizations in the virtual scroller @skjnldsv
There are comments on the individual changes. Again, it's hard to measure performance here.