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

Revert "Remove the "ancient PC" optimization" #217

Merged
merged 1 commit into from
Oct 6, 2023
Merged

Conversation

Borewit
Copy link
Owner

@Borewit Borewit commented Oct 5, 2023

This partially reverts commit da15e81 (PR #209).

Reason for reversing this PR is degraded performance reported here: #214 (comment).

  • Restores the ancient PC optimization
  • But keeps the change which prevent reconstructing the MatchedPlaylistEntryComparator

@Borewit
Copy link
Owner Author

Borewit commented Oct 5, 2023

@touwys, can you confirm that this reverses the 30% performance loss you reported here?

@Borewit Borewit self-assigned this Oct 5, 2023
@Borewit Borewit force-pushed the revert-pr-209 branch 2 times, most recently from 7945575 to 2acfee2 Compare October 5, 2023 18:06
Repository owner deleted a comment from github-actions bot Oct 5, 2023
Repository owner deleted a comment from github-actions bot Oct 5, 2023
This partially reverts commit da15e81.

- Restores the ancient PC optimization
- But prevent reconstructing the `MatchedPlaylistEntryComparator`
Repository owner deleted a comment from github-actions bot Oct 5, 2023
@github-actions
Copy link

github-actions bot commented Oct 5, 2023

@touwys
Copy link

touwys commented Oct 6, 2023

@Borewit :

listFix()-2.8.1-16-4a8253b

Reverse 'Ancient PC' Playlist Repair Speed Optimization

Test Results

Note: In the table below, the previous repair speed test results are displayed in parentheses.

REPAIR ACTION TEST-1: FLAC+MP3 TEST-2: MP3
Find Exact Matches 1m 08s (1m 08s) 0m 29s (0m 29s)
Find Closest Matches 3m 40s (5m 17s) 1m 54s (2m 42s)
Find All Matches 4m 48s (6m 26s) 2m 24s (3m 11s)

👉🏻 Good going. The result is positive, and consistent with those previously evident. Noticeable — even a slight gain in overall playlist repair speed.


Additional Observations & Comments

I'm unable to locate those "logfile.log" files again... did the name, or location, perhaps change at all?

No change, should be in: %USERPROFILE%.listFix()\logs, e.g. C:\Users\touwys.listFix()\logs 👈🏻

  1. Still cannot locate the \.listfix() "logs" folder anywhere:

    Screenshot

  2. Did you spot any difficulty concerning the app memory management, which I mentioned in an earlier report?


@Borewit
Copy link
Owner Author

Borewit commented Oct 6, 2023

Did you spot any difficulty concerning the app memory management, which I mentioned in an earlier report?

If open a broken playlist, repair it, close that playlist without saving the changes and repeat that a few times, I do see an increase in memory usage.

@Borewit Borewit merged commit a752070 into main Oct 6, 2023
4 checks passed
@Borewit Borewit added the internal Excluded from release notes label Oct 6, 2023
@Borewit Borewit deleted the revert-pr-209 branch October 23, 2023 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Excluded from release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants