From ecd0740c7d13e2684d541994d7f57df936e40aea Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Mon, 25 Apr 2022 17:16:47 -0700 Subject: [PATCH] MediaGallery: remove "isSubstantialRequest" optimization MediaGallery tried to avoid loading just a few items at a time, because database queries are expensive and it's better to do work in bulk. But the logic to do so was complicated and not entirely correct (swiping very quickly through the detail view could run out of loaded items while still considering the "next few" to be "not substantial"). Just remove it. --- .../MediaGallery/MediaGallery.swift | 80 ------------------- 1 file changed, 80 deletions(-) diff --git a/Signal/src/ViewControllers/MediaGallery/MediaGallery.swift b/Signal/src/ViewControllers/MediaGallery/MediaGallery.swift index 559cf8c4bc..debb109acc 100644 --- a/Signal/src/ViewControllers/MediaGallery/MediaGallery.swift +++ b/Signal/src/ViewControllers/MediaGallery/MediaGallery.swift @@ -393,86 +393,6 @@ class MediaGallery: Dependencies { return range }() - func isSubstantialRequest() -> Bool { - // If we have not loaded the item at the given path yet, it's substantial. - guard let item = anchorItem else { - return true - } - - let sectionItems = sections[sectionIndex].value - - // If we're loading the remainder of an album, check to see if any items in the album are not loaded yet. - if shouldLoadAlbumRemainder { - let albumStart = (itemIndex - item.albumIndex) - .clamp(sectionItems.startIndex, sectionItems.endIndex) - let albumEnd = (albumStart + item.message.attachmentIds.count) - .clamp(sectionItems.startIndex, sectionItems.endIndex) - if sectionItems[albumStart..) -> Int { - return slice.lazy.filter { $0 == nil }.count - } - - owsAssertDebug(naiveRequestRange.lowerBound < sectionItems.count) - let sectionSliceStart = naiveRequestRange.lowerBound - .clamp(sectionItems.startIndex, sectionItems.endIndex) - let sectionSliceEnd = naiveRequestRange.upperBound - .clamp(sectionItems.startIndex, sectionItems.endIndex) - let sectionSlice = sectionItems[sectionSliceStart.. sectionItems.count { - var currentSectionIndex = sectionIndex + 1 - var remainingForward = naiveRequestRange.upperBound - sectionItems.count - repeat { - guard let currentSectionItems = sections[safe: currentSectionIndex]?.value else { - // We've reached the end of the fetched sections. If there are more sections, or the last item - // isn't fetched yet, assume it's substantial. - if !hasFetchedMostRecent || (sections.last?.value.last ?? nil) == nil { - return true - } - break - } - unfetchedCount += countUnfetched(in: currentSectionItems.prefix(remainingForward)) - if remainingForward <= currentSectionItems.count { - break - } - remainingForward -= currentSectionItems.count - currentSectionIndex += 1 - } while true - } - - if naiveRequestRange.lowerBound < 0 { - var currentSectionIndex = sectionIndex - 1 - var remainingBackward = -naiveRequestRange.lowerBound - repeat { - guard let currentSectionItems = sections[safe: currentSectionIndex]?.value else { - // We've reached the start of the fetched sections. If there are more sections, or the first - // item isn't fetched yet, assume it's substantial. - if !hasFetchedOldest || (sections.first?.value.first ?? nil) == nil { - return true - } - break - } - unfetchedCount += countUnfetched(in: currentSectionItems.suffix(remainingBackward)) - if remainingBackward <= currentSectionItems.count { - break - } - remainingBackward -= currentSectionItems.count - currentSectionIndex -= 1 - } while true - } - - // If we haven't hit the start or end, and more than half the items are unfetched, it's substantial. - return unfetchedCount > (naiveRequestRange.count / 2) - } - - guard isSubstantialRequest() else { - return - } - var numNewlyLoadedEarlierSections: Int = 0 var numNewlyLoadedLaterSections: Int = 0