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.

Sign up for GitHub

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

Jump to bottom

Improve history import performance and fix some bugs #5666

Merged
merged 5 commits into from
Sep 16, 2024
Merged

Improve history import performance and fix some bugs #5666

merged 5 commits into from
Sep 16, 2024

Conversation

absidue
Copy link
Member

@absidue absidue commented Sep 8, 2024 •
edited
Loading

Improve history import performance and fix some bugs

Pull Request Type

  • Bugfix
  • Performance improvement

Related issue

closes #5174

Description

The current history import has a few flaws, which this pull request aims to iron out:

  • It is slow and shows the import finished toast way before the import is actually done (in my case minutes earlier), because it shows the toast when it has triggered the asynchronous operations, instead of waiting for them to finish
  • Because it uses the same mark as watched logic as the watch page, each item that gets imported is moved to the start of the history array, regardless of whether that is the correct chronological position or not (the data is correct in the database but it means that until you restart FreeTube the history tab shows the history items in the wrong order)

This pull request implements dedicated logic for the history import which merges together the existing history and the imported history with the help of a Map, then overwrites the historyCacheSorted and historyCacheById properties in the store and runs delete all and insert all on the database (remember we've already merged together the histories), which is a lot faster than an insert or update for every single history entry.

Testing

Back up your history first

Test cases:

  1. Import your history into an empty FreeTube/clear your history first before importing (testing how it handles everything being new)
  2. Import your history again, this time without deleting your existing history (testing how it handles every imported item already existing)
  3. Mark a few videos as watched that aren't in your history yet, import your history again (testing that it retains existing items that weren't in the import).

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 1a36dab

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Sep 8, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) September 8, 2024 21:19
@PikachuEXE
Copy link
Collaborator

Found an issue when testing: viewCount required which was exported after I imported from YT history
image

Not sure if you want to do anything about that in this PR too

Got 16K entries can't be imported due to this so can't really test the speed yet

@FreeTubeBot FreeTubeBot merged commit 01dbdad into FreeTubeApp:development Sep 16, 2024
5 checks passed
@absidue absidue deleted the history-import-improvements branch September 16, 2024 11:14
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Sep 16, 2024
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Sep 16, 2024
@PikachuEXE
Merge branch 'development' into feature/subscription-cache
8ef7b92 
* development: (29 commits)
  Translated using Weblate (Flemish (West))
  Added translation using Weblate (Flemish (West))
  Translated using Weblate (Slovak)
  Improve history import performance and fix some bugs (FreeTubeApp#5666)
  Bump electron from 32.0.2 to 32.1.0 (FreeTubeApp#5710)
  Local API: Use IOS HLS manifest for livestreams (FreeTubeApp#5705)
  Translated using Weblate (Slovak)
  Translated using Weblate (Japanese)
  Bump the stylelint group with 2 updates (FreeTubeApp#5706)
  Bump swiper from 11.1.12 to 11.1.14 (FreeTubeApp#5709)
  Fix a few memory leaks while tearing down the player (FreeTubeApp#5698)
  Fix audio track selection (FreeTubeApp#5697)
  Bump shaka-player from 4.10.12 to 4.11.1 (FreeTubeApp#5677)
  Translated using Weblate (Czech)
  Translated using Weblate (Serbian)
  Translated using Weblate (Polish)
  Translated using Weblate (German)
  Translated using Weblate (Chinese (Simplified Han script))
  Bump express from 4.19.2 to 4.20.0 (FreeTubeApp#5687)
  Translated using Weblate (Turkish)
  ...
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Sep 16, 2024
@PikachuEXE
Merge branch 'development' into feature/history/remember-search-query
47222b9 
* development: (74 commits)
  Translated using Weblate (Flemish (West))
  Added translation using Weblate (Flemish (West))
  Translated using Weblate (Slovak)
  Improve history import performance and fix some bugs (FreeTubeApp#5666)
  Bump electron from 32.0.2 to 32.1.0 (FreeTubeApp#5710)
  Local API: Use IOS HLS manifest for livestreams (FreeTubeApp#5705)
  Translated using Weblate (Slovak)
  Translated using Weblate (Japanese)
  Bump the stylelint group with 2 updates (FreeTubeApp#5706)
  Bump swiper from 11.1.12 to 11.1.14 (FreeTubeApp#5709)
  Fix a few memory leaks while tearing down the player (FreeTubeApp#5698)
  Fix audio track selection (FreeTubeApp#5697)
  Bump shaka-player from 4.10.12 to 4.11.1 (FreeTubeApp#5677)
  Translated using Weblate (Czech)
  Translated using Weblate (Serbian)
  Translated using Weblate (Polish)
  Translated using Weblate (German)
  Translated using Weblate (Chinese (Simplified Han script))
  Bump express from 4.19.2 to 4.20.0 (FreeTubeApp#5687)
  Translated using Weblate (Turkish)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChunkyProgrammer ChunkyProgrammer ChunkyProgrammer approved these changes

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc approved these changes

@PikachuEXE PikachuEXE PikachuEXE approved these changes

@kommunarr kommunarr Awaiting requested review from kommunarr

@MarmadileManteater MarmadileManteater Awaiting requested review from MarmadileManteater

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: History import ends with error messages, successful but misses ~2 years
5 participants
@absidue @PikachuEXE @efb4f5ff-1298-471a-8973-3d47447115dc @ChunkyProgrammer @FreeTubeBot