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

Improve performance of the watch history handling #4017

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/renderer/components/data-settings/data-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ export default defineComponent({
allPlaylists: function () {
return this.$store.getters.getAllPlaylists
},
historyCache: function () {
return this.$store.getters.getHistoryCache
historyCacheSorted: function () {
return this.$store.getters.getHistoryCacheSorted
},
exportSubscriptionsPromptNames: function () {
const exportFreeTube = this.$t('Settings.Data Settings.Export FreeTube')
Expand Down Expand Up @@ -825,7 +825,7 @@ export default defineComponent({
},

exportHistory: async function () {
const historyDb = this.historyCache.map((historyEntry) => {
const historyDb = this.historyCacheSorted.map((historyEntry) => {
return JSON.stringify(historyEntry)
}).join('\n') + '\n'
const dateStr = getTodayDateStrLocalTimezone()
Expand Down
42 changes: 20 additions & 22 deletions src/renderer/components/ft-list-video/ft-list-video.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,12 @@ export default defineComponent({
}
},
computed: {
historyCache: function () {
return this.$store.getters.getHistoryCache
historyEntry: function () {
return this.$store.getters.getHistoryCacheById[this.id]
},
absidue marked this conversation as resolved.
Show resolved Hide resolved

historyEntryExists: function () {
return typeof this.historyEntry !== 'undefined'
},

listType: function () {
Expand Down Expand Up @@ -313,12 +317,6 @@ export default defineComponent({
}
},

historyIndex: function() {
return this.historyCache.findIndex((video) => {
return video.videoId === this.id
})
},

playlistIdFinal: function () {
if (this.playlistId) {
return this.playlistId
Expand All @@ -327,12 +325,8 @@ export default defineComponent({
// Get playlist ID from history ONLY if option enabled
if (!this.showVideoWithLastViewedPlaylist) { return }
if (!this.saveVideoHistoryWithLastViewedPlaylist) { return }
const historyIndex = this.historyIndex
if (historyIndex === -1) {
return undefined
}

return this.historyCache[historyIndex].lastViewedPlaylistId
return this.historyEntry?.lastViewedPlaylistId
},

currentLocale: function () {
Expand All @@ -348,7 +342,7 @@ export default defineComponent({
}
},
watch: {
historyIndex() {
historyEntry() {
this.checkIfWatched()
},
},
Expand Down Expand Up @@ -451,8 +445,8 @@ export default defineComponent({
this.channelName = this.data.author ?? null
this.channelId = this.data.authorId ?? null

if (this.data.isRSS && this.historyIndex !== -1) {
this.duration = formatDurationAsTimestamp(this.historyCache[this.historyIndex].lengthSeconds)
if (this.data.isRSS && typeof this.historyEntryExists) {
absidue marked this conversation as resolved.
Show resolved Hide resolved
absidue marked this conversation as resolved.
Show resolved Hide resolved
this.duration = formatDurationAsTimestamp(this.historyEntry.lengthSeconds)
} else {
this.duration = formatDurationAsTimestamp(this.data.lengthSeconds)
}
Expand Down Expand Up @@ -538,22 +532,26 @@ export default defineComponent({
},

checkIfWatched: function () {
const historyIndex = this.historyIndex

if (historyIndex !== -1) {
if (this.historyEntryExists) {
this.watched = true

const historyEntry = this.historyEntry

if (this.saveWatchedProgress) {
// For UX consistency, no progress reading if writing disabled
this.watchProgress = this.historyCache[historyIndex].watchProgress
this.watchProgress = historyEntry.watchProgress
}

if (this.historyCache[historyIndex].published !== '') {
const videoPublished = this.historyCache[historyIndex].published
if (historyEntry.published !== '') {
const videoPublished = historyEntry.published
const videoPublishedDate = new Date(videoPublished)
this.publishedText = videoPublishedDate.toLocaleDateString()
} else {
this.publishedText = ''
}
} else {
this.watched = false
this.watchProgress = 0
}
},

Expand Down
8 changes: 2 additions & 6 deletions src/renderer/helpers/subscriptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,10 @@ export function updateVideoListAfterProcessing(videos) {
}

if (store.getters.getHideWatchedSubs) {
const historyCache = store.getters.getHistoryCache
const historyCacheById = store.getters.getHistoryCacheById

videoList = videoList.filter((video) => {
const historyIndex = historyCache.findIndex((x) => {
return x.videoId === video.videoId
})

return historyIndex === -1
return !Object.hasOwn(historyCacheById, video.videoId)
})
}

Expand Down
65 changes: 43 additions & 22 deletions src/renderer/store/modules/history.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,36 @@
import { set as vueSet, del as vueDel } from 'vue'
import { DBHistoryHandlers } from '../../../datastores/handlers/index'

const state = {
historyCache: []
historyCacheSorted: [],

// Vuex doesn't support Maps, so we have to use an object here instead
// TODO: switch to a Map during the Pinia migration
historyCacheById: {}
}

const getters = {
getHistoryCache: () => {
return state.historyCache
getHistoryCacheSorted: () => {
return state.historyCacheSorted
},

getHistoryCacheById: () => {
return state.historyCacheById
}
}

const actions = {
async grabHistory({ commit }) {
try {
const results = await DBHistoryHandlers.find()
commit('setHistoryCache', results)

const resultsById = {}
results.forEach(video => {
resultsById[video.videoId] = video
})

commit('setHistoryCacheSorted', results)
commit('setHistoryCacheById', resultsById)
} catch (errMessage) {
console.error(errMessage)
}
Expand All @@ -41,7 +57,8 @@ const actions = {
async removeAllHistory({ commit }) {
try {
await DBHistoryHandlers.deleteAll()
commit('setHistoryCache', [])
commit('setHistoryCacheSorted', [])
commit('setHistoryCacheById', {})
} catch (errMessage) {
console.error(errMessage)
}
Expand All @@ -67,56 +84,60 @@ const actions = {
}

const mutations = {
setHistoryCache(state, historyCache) {
state.historyCache = historyCache
setHistoryCacheSorted(state, historyCacheSorted) {
state.historyCacheSorted = historyCacheSorted
},

hoistEntryToTopOfHistoryCache(state, { currentIndex, updatedEntry }) {
state.historyCache.splice(currentIndex, 1)
state.historyCache.unshift(updatedEntry)
setHistoryCacheById(state, historyCacheById) {
state.historyCacheById = historyCacheById
},

upsertToHistoryCache(state, record) {
const i = state.historyCache.findIndex((currentRecord) => {
const i = state.historyCacheSorted.findIndex((currentRecord) => {
return record.videoId === currentRecord.videoId
})

if (i !== -1) {
// Already in cache
// Must be hoisted to top, remove it and then unshift it
state.historyCache.splice(i, 1)
state.historyCacheSorted.splice(i, 1)
}

state.historyCache.unshift(record)
state.historyCacheSorted.unshift(record)
vueSet(state.historyCacheById, record.videoId, record)
},

updateRecordWatchProgressInHistoryCache(state, { videoId, watchProgress }) {
const i = state.historyCache.findIndex((currentRecord) => {
const i = state.historyCacheSorted.findIndex((currentRecord) => {
return currentRecord.videoId === videoId
})

const targetRecord = Object.assign({}, state.historyCache[i])
const targetRecord = Object.assign({}, state.historyCacheSorted[i])
targetRecord.watchProgress = watchProgress
state.historyCache.splice(i, 1, targetRecord)
state.historyCacheSorted.splice(i, 1, targetRecord)
vueSet(state.historyCacheById, videoId, targetRecord)
},

updateRecordLastViewedPlaylistIdInHistoryCache(state, { videoId, lastViewedPlaylistId }) {
const i = state.historyCache.findIndex((currentRecord) => {
const i = state.historyCacheSorted.findIndex((currentRecord) => {
return currentRecord.videoId === videoId
})

const targetRecord = Object.assign({}, state.historyCache[i])
const targetRecord = Object.assign({}, state.historyCacheSorted[i])
targetRecord.lastViewedPlaylistId = lastViewedPlaylistId
state.historyCache.splice(i, 1, targetRecord)
state.historyCacheSorted.splice(i, 1, targetRecord)
vueSet(state.historyCacheById, videoId, targetRecord)
},

removeFromHistoryCacheById(state, videoId) {
for (let i = 0; i < state.historyCache.length; i++) {
if (state.historyCache[i].videoId === videoId) {
state.historyCache.splice(i, 1)
for (let i = 0; i < state.historyCacheSorted.length; i++) {
if (state.historyCacheSorted[i].videoId === videoId) {
state.historyCacheSorted.splice(i, 1)
break
}
}

vueDel(state.historyCacheById, videoId)
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/renderer/store/modules/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,8 @@ const customActions = {
break

case SyncEvents.GENERAL.DELETE_ALL:
commit('setHistoryCache', [])
commit('setHistoryCacheSorted', [])
commit('setHistoryCacheById', {})
break

default:
Expand Down
16 changes: 8 additions & 8 deletions src/renderer/views/History/History.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ export default defineComponent({
}
},
computed: {
historyCache: function () {
return this.$store.getters.getHistoryCache
historyCacheSorted: function () {
return this.$store.getters.getHistoryCacheSorted
},

fullData: function () {
if (this.historyCache.length < this.dataLimit) {
return this.historyCache
if (this.historyCacheSorted.length < this.dataLimit) {
return this.historyCacheSorted
} else {
return this.historyCache.slice(0, this.dataLimit)
return this.historyCacheSorted.slice(0, this.dataLimit)
}
}
},
Expand All @@ -59,7 +59,7 @@ export default defineComponent({

this.activeData = this.fullData

if (this.activeData.length < this.historyCache.length) {
if (this.activeData.length < this.historyCacheSorted.length) {
this.showLoadMoreButton = true
} else {
this.showLoadMoreButton = false
Expand All @@ -85,14 +85,14 @@ export default defineComponent({
filterHistory: function() {
if (this.query === '') {
this.activeData = this.fullData
if (this.activeData.length < this.historyCache.length) {
if (this.activeData.length < this.historyCacheSorted.length) {
this.showLoadMoreButton = true
} else {
this.showLoadMoreButton = false
}
} else {
const lowerCaseQuery = this.query.toLowerCase()
const filteredQuery = this.historyCache.filter((video) => {
const filteredQuery = this.historyCacheSorted.filter((video) => {
if (typeof (video.title) !== 'string' || typeof (video.author) !== 'string') {
return false
} else {
Expand Down
19 changes: 9 additions & 10 deletions src/renderer/views/Watch/Watch.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,11 @@ export default defineComponent({
}
},
computed: {
historyCache: function () {
return this.$store.getters.getHistoryCache
historyEntry: function () {
return this.$store.getters.getHistoryCacheById[this.videoId]
},
absidue marked this conversation as resolved.
Show resolved Hide resolved
historyEntryExists: function () {
return typeof this.historyEntry !== 'undefined'
},
rememberHistory: function () {
return this.$store.getters.getRememberHistory
Expand Down Expand Up @@ -1040,10 +1043,6 @@ export default defineComponent({
},

checkIfWatched: function () {
const historyIndex = this.historyCache.findIndex((video) => {
return video.videoId === this.videoId
})

if (!this.isLive) {
if (this.timestamp) {
if (this.timestamp < 0) {
Expand All @@ -1053,9 +1052,9 @@ export default defineComponent({
} else {
this.$refs.videoPlayer.player.currentTime(this.timestamp)
}
} else if (this.saveWatchedProgress && historyIndex !== -1) {
} else if (this.saveWatchedProgress && this.historyEntryExists) {
// For UX consistency, no progress reading if writing disabled
const watchProgress = this.historyCache[historyIndex].watchProgress
const watchProgress = this.historyEntry.watchProgress

if (watchProgress < (this.videoLengthSeconds - 10)) {
this.$refs.videoPlayer.player.currentTime(watchProgress)
Expand All @@ -1066,8 +1065,8 @@ export default defineComponent({
if (this.rememberHistory) {
if (this.timestamp) {
this.addToHistory(this.timestamp)
} else if (historyIndex !== -1) {
this.addToHistory(this.historyCache[historyIndex].watchProgress)
} else if (this.historyEntryExists) {
this.addToHistory(this.historyEntry.watchProgress)
} else {
this.addToHistory(0)
}
Expand Down