Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
13 changes: 11 additions & 2 deletions data/src/main/java/org/monogram/data/datasource/FileDataSource.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,23 @@ import org.drinkless.tdlib.TdApi
import org.monogram.data.infra.FileDownloadQueue

interface FileDataSource {
suspend fun downloadFile(fileId: Int, priority: Int, offset: Long, limit: Long, synchronous: Boolean): TdApi.File?
suspend fun downloadFile(
fileId: Int,
priority: Int,
offset: Long,
limit: Long,
synchronous: Boolean,
type: FileDownloadQueue.DownloadType
userInitiated: Boolean = false
): TdApi.File?

suspend fun downloadFile(
fileId: Int,
priority: Int,
offset: Long,
limit: Long,
synchronous: Boolean,
type: FileDownloadQueue.DownloadType,
userInitiated: Boolean = false
): TdApi.File?
suspend fun cancelDownload(fileId: Int): TdApi.Ok?
suspend fun getFile(fileId: Int): TdApi.File?
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package org.monogram.data.datasource

import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.withTimeout
import org.drinkless.tdlib.TdApi
import org.monogram.data.core.coRunCatching
import org.monogram.data.gateway.TelegramGateway
import org.monogram.data.infra.FileDownloadQueue

private const val SYNCHRONOUS_DOWNLOAD_TIMEOUT_MS = 60_000L

class TdFileDataSource(
private val gateway: TelegramGateway,
private val fileDownloadQueue: FileDownloadQueue
Expand All @@ -15,15 +18,17 @@ class TdFileDataSource(
priority: Int,
offset: Long,
limit: Long,
synchronous: Boolean
synchronous: Boolean,
userInitiated: Boolean
): TdApi.File? {
return downloadFile(
fileId = fileId,
priority = priority,
offset = offset,
limit = limit,
synchronous = synchronous,
type = FileDownloadQueue.DownloadType.DEFAULT
type = FileDownloadQueue.DownloadType.DEFAULT,
userInitiated = userInitiated
)
}

Expand All @@ -33,7 +38,8 @@ class TdFileDataSource(
offset: Long,
limit: Long,
synchronous: Boolean,
type: FileDownloadQueue.DownloadType
type: FileDownloadQueue.DownloadType,
userInitiated: Boolean
): TdApi.File? {
fileDownloadQueue.clearSuppression(fileId)
fileDownloadQueue.enqueue(
Expand All @@ -43,10 +49,17 @@ class TdFileDataSource(
offset,
limit,
synchronous,
ignoreSuppression = true
ignoreSuppression = true,
userInitiated = userInitiated
)
if (synchronous) {
coRunCatching { fileDownloadQueue.waitForDownload(fileId).await() }
// Bounded: the queue can legitimately decline to enqueue (cooldown, suppression),
// and an unbounded await here left story loads hanging forever when it did.
coRunCatching {
withTimeout(SYNCHRONOUS_DOWNLOAD_TIMEOUT_MS) {
fileDownloadQueue.waitForDownload(fileId).await()
}
}
}
return getFile(fileId)
}
Expand Down
97 changes: 71 additions & 26 deletions data/src/main/java/org/monogram/data/infra/FileDownloadQueue.kt
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,11 @@ class FileDownloadQueue(
if (this.isManual != other.isManual) return if (this.isManual) -1 else 1
val p = other.priority.compareTo(priority)
if (p != 0) return p
val a = availableAt.compareTo(other.availableAt)
return if (a != 0) a else createdAt.compareTo(other.createdAt)
// Match TDLib: among equal priorities the most recent request wins (LIFO).
// `availableAt` is deliberately not a ranking key -- it expresses readiness
// (backoff/cooldown) and is already enforced as a filter in dispatchTasks().
// Ranking by it would keep this ordering FIFO and defeat the recency rule.
return other.createdAt.compareTo(createdAt)
}
}

Expand All @@ -76,17 +79,19 @@ class FileDownloadQueue(

@Volatile
private var activeChatId: Long = 0L
@Volatile
private var lastTaskStartAt: Long = 0L

private val startGapMs = 160L
// TDLib has no concurrent-download cap of its own: it self-regulates with a 2 MiB in-flight
// byte budget per (DC, size-class) and orders equal priorities LIFO. Keeping these caps tight
// only withholds requests from that scheduler, so they are sized to stay out of its way.
private val notFoundCooldownMs = TimeUnit.MINUTES.toMillis(2)
private val maxTotalParallelDownloads = 10
private val maxVideoParallelDownloads = 3
private val maxGifParallelDownloads = 2
private val maxDefaultParallelDownloads = 4
private val maxPendingDefaultAutoDownloads = 10
private val maxStickerParallelDownloads = 5
private val maxTotalParallelDownloads = 48
// Video stays low deliberately: a large file monopolises TDLib's byte window anyway, so
// extra video slots add queueing without throughput.
private val maxVideoParallelDownloads = 4
private val maxGifParallelDownloads = 4
private val maxDefaultParallelDownloads = 32
private val maxPendingDefaultAutoDownloads = 64
private val maxStickerParallelDownloads = 16
private val stickerStallMs = 20_000L
private val defaultStallMs = 35_000L
private val stalledRecoveryCooldownMs = 12_000L
Expand Down Expand Up @@ -176,19 +181,22 @@ class FileDownloadQueue(
}
}

// No inter-start gap: awaiting one here serialised every start and, because `trigger` is
// CONFLATED, an urgent enqueue arriving mid-dispatch could have its wake-up coalesced
// away. TDLib already paces part issuance itself (DelayDispatcher, 50ms -> 3ms ramp).
for (task in tasksToStart) {
throttleTaskStart()
scope.launch(dispatcherProvider.io) {
processDownload(task)
}
}
}

private suspend fun throttleTaskStart() {
val now = System.currentTimeMillis()
val waitMs = (lastTaskStartAt + startGapMs - now).coerceAtLeast(0L)
if (waitMs > 0) delay(waitMs)
lastTaskStartAt = System.currentTimeMillis()
// A conflated trigger can drop a wake-up that arrived while we were dispatching, which
// would strand ready work until the 15s stall sweep. Re-arm only when this pass actually
// started something and more is waiting -- gating on progress keeps this from spinning
// when the backlog is blocked purely by slot exhaustion.
if (tasksToStart.isNotEmpty() && pendingRequests.isNotEmpty()) {
trigger.trySend(Unit)
}
}

private suspend fun processDownload(req: DownloadRequest) {
Expand Down Expand Up @@ -535,14 +543,18 @@ class FileDownloadQueue(
offset: Long = 0,
limit: Long = 0,
synchronous: Boolean = false,
ignoreSuppression: Boolean = false
ignoreSuppression: Boolean = false,
userInitiated: Boolean = false
) {
scope.launch(dispatcherProvider.default) {
if (!ignoreSuppression && suppressedAutoDownloadIds.contains(fileId)) {
return@launch
}

val isManualRequest = priority >= 32
// User intent is an explicit signal, not something inferred from a magic priority.
// It used to be `priority >= 32`, but viewport auto-download also passes 32, so every
// thumbnail scrolled past was marked "manual" and became un-evictable.
val isManualRequest = userInitiated
if (isManualRequest) manualDownloadIds.add(fileId)

val cooldownUntil = notFoundCooldownUntil[fileId]
Expand Down Expand Up @@ -592,7 +604,13 @@ class FileDownloadQueue(
val active = activeRequests[fileId]
if (active != null) {
val merged = mergeRequests(active, req)
val shouldKick = merged != active || cache.fileCache[fileId]?.local?.isDownloadingActive != true
// Always re-send on user action. FileManager::download_impl runs with
// force_update_priority=true and ResourceManager::add_node inserts before
// equals, so re-issuing DownloadFile -- even at an unchanged priority --
// moves the file to the head of TDLib's queue. It is a free bump-to-front.
val shouldKick = userInitiated ||
merged != active ||
cache.fileCache[fileId]?.local?.isDownloadingActive != true
if (shouldKick) {
activeRequests[fileId] = merged
scope.launch(dispatcherProvider.io) {
Expand All @@ -613,15 +631,33 @@ class FileDownloadQueue(
return@launch
}

// A genuinely user-initiated request must reach TDLib immediately rather than
// queueing behind background work. TDLib imposes no concurrency cap and orders
// equal priorities LIFO, so handing it straight over makes the newest action win.
// This intentionally lets activeRequests exceed maxTotalParallelDownloads;
// dispatchTasks() recomputes its counters per pass and simply admits nothing new
// until the set drains.
if (userInitiated) {
pendingRequests.remove(fileId)
activeRequests[fileId] = req
scope.launch(dispatcherProvider.io) {
processDownload(req)
}
return@launch
}

val pending = pendingRequests[fileId]
if (pending != null) {
pendingRequests[fileId] = mergeRequests(pending, req)
} else {
if (!isManual && resolvedType == DownloadType.DEFAULT) {
// Never silently drop a synchronous request: callers await waitForDownload(),
// and abandoning it here leaves that deferred uncompleted forever.
if (!isManual && !synchronous && resolvedType == DownloadType.DEFAULT) {
val pendingDefaultCount = pendingRequests.values.count {
!it.isManual && it.type == DownloadType.DEFAULT
}
if (req.priority < 32 && pendingDefaultCount >= maxPendingDefaultAutoDownloads) {
downloadWaiters.remove(fileId)?.cancel()
return@launch
}
}
Expand Down Expand Up @@ -714,8 +750,11 @@ class FileDownloadQueue(
var max = 1
val type = fileDownloadTypes[fileId]

// Priority only does work when values differ. These used to saturate at 32 -- the same
// value as an explicit user action -- which left TDLib nothing to arbitrate and reduced
// every scheduling decision to the tie-break. 32 is now reserved for real user intent.
if (type == DownloadType.STICKER || type == DownloadType.VIDEO_NOTE) {
max = 32
max = 8
}

messages.forEach { (chatId, msgId) ->
Expand All @@ -724,9 +763,9 @@ class FileDownloadQueue(

var p = 1
if (isVisible) {
p = if (chatId == activeChatId) 32 else 24
} else if (isNearby) {
p = if (chatId == activeChatId) 16 else 8
} else if (isNearby) {
p = if (chatId == activeChatId) 4 else 2
}

max = maxOf(max, p)
Expand Down Expand Up @@ -755,11 +794,17 @@ class FileDownloadQueue(

private fun cancelIrrelevantDownloads() {
scope.launch(dispatcherProvider.default) {
val toCancel = mutableListOf<Int>()
// Scan active downloads too, not just pending ones. An in-flight download that the
// user has scrolled past used to hold its slot until completion or the 3-minute
// timeout, which is what let stale work block foreground requests.
val toCancel = LinkedHashSet<Int>()

for ((fileId, _) in pendingRequests) {
if (!isStillRelevant(fileId)) toCancel.add(fileId)
}
for ((fileId, _) in activeRequests) {
if (!isStillRelevant(fileId)) toCancel.add(fileId)
}

toCancel.forEach { fileId -> cancelDownload(fileId, force = false, suppress = false) }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -896,9 +896,16 @@ internal class MessageRepositoryImpl(
}
}

override fun downloadFile(fileId: Int, priority: Int, offset: Long, limit: Long, synchronous: Boolean) {
override fun downloadFile(
fileId: Int,
priority: Int,
offset: Long,
limit: Long,
synchronous: Boolean,
userInitiated: Boolean
) {
scope.launch {
fileDataSource.downloadFile(fileId, priority, offset, limit, synchronous)
fileDataSource.downloadFile(fileId, priority, offset, limit, synchronous, userInitiated)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,11 @@ class StoryRepositoryImpl(
priority = priority,
offset = 0,
limit = 0,
synchronous = synchronous
synchronous = synchronous,
// Opening a story is an explicit user action. Story files are not registered
// per-message, so calculatePriority() cannot lift them -- without this they rank
// below every scrolled-past thumbnail and are the only work the drop guard sheds.
userInitiated = true
)
}.onFailure {
Log.d(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ interface FileRepository {
priority: Int = 1,
offset: Long = 0,
limit: Long = 0,
synchronous: Boolean = false
synchronous: Boolean = false,
userInitiated: Boolean = false
)

suspend fun cancelDownloadFile(fileId: Int)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ interface ChatComponent {
fun onViewportSettled()
fun onMessageViewportChanged(visibleMessageIds: Set<Long>, nearbyMessageIds: Set<Long>)
fun onScrollToBottom()
fun onDownloadFile(fileId: Int)
fun onDownloadFile(fileId: Int, userInitiated: Boolean = false)
fun onDownloadHighRes(messageId: Long)
fun onCancelDownloadFile(fileId: Int)
fun updateScrollPosition(messageId: Long)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ interface ChatStore : Store<ChatStore.Intent, ChatComponent.State, ChatStore.Lab
object ScrollToMessageConsumed : Intent()
object ScrollCommandConsumed : Intent()
object ScrollToBottom : Intent()
data class DownloadFile(val fileId: Int) : Intent()
data class DownloadFile(val fileId: Int, val userInitiated: Boolean = false) : Intent()
data class DownloadHighRes(val messageId: Long) : Intent()
data class CancelDownloadFile(val fileId: Int) : Intent()
data class UpdateScrollPosition(val messageId: Long) : Intent()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ class ChatStoreFactory(
else it.copy(scrollToMessageId = null, pendingScrollCommand = null)
}
is Intent.ScrollToBottom -> component.scrollToBottomInternal()
is Intent.DownloadFile -> component.handleDownloadFile(intent.fileId)
is Intent.DownloadFile -> component.handleDownloadFile(intent.fileId, intent.userInitiated)
is Intent.DownloadHighRes -> component.handleDownloadHighRes(intent.messageId)

is Intent.CancelDownloadFile -> component.handleCancelDownloadFile(intent.fileId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -876,9 +876,9 @@ class DefaultChatComponent(

override fun onScrollToBottom() = store.accept(ChatStore.Intent.ScrollToBottom)

override fun onDownloadFile(fileId: Int) {
override fun onDownloadFile(fileId: Int, userInitiated: Boolean) {
AutoDownloadSuppression.clear(fileId)
store.accept(ChatStore.Intent.DownloadFile(fileId))
store.accept(ChatStore.Intent.DownloadFile(fileId, userInitiated))
}

override fun onDownloadHighRes(messageId: Long) = store.accept(ChatStore.Intent.DownloadHighRes(messageId))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,15 @@ import kotlinx.coroutines.launch
import org.monogram.domain.models.MessageContent
import org.monogram.presentation.features.chats.conversation.DefaultChatComponent

internal fun DefaultChatComponent.handleDownloadFile(fileId: Int) {
repositoryMessage.downloadFile(fileId, priority = 32)
internal fun DefaultChatComponent.handleDownloadFile(fileId: Int, userInitiated: Boolean = false) {
// Only an explicit gesture gets priority 32 and the user-initiated fast path. Viewport
// prefetch used to pass 32 as well, which marked every scrolled-past thumbnail as "manual"
// and left it un-evictable, silting up the download slots.
repositoryMessage.downloadFile(
fileId,
priority = if (userInitiated) 32 else 16,
userInitiated = userInitiated
)
}

internal fun DefaultChatComponent.handleCancelDownloadFile(fileId: Int) {
Expand All @@ -25,7 +32,10 @@ internal fun DefaultChatComponent.handleDownloadHighRes(messageId: Long) {
val fileId = repositoryMessage.getHighResFileId(chatId, messageId)
if (fileId != null) {
updatePhotoOriginalFileId(messageId, fileId)
repositoryMessage.downloadFile(fileId, priority = 32)
// Opening a photo full screen is an unambiguous user action: this file is usually
// cold (only the "x" size was ever prefetched), so it needs the bypass to reach
// TDLib immediately rather than queueing behind background thumbnails.
repositoryMessage.downloadFile(fileId, priority = 32, userInitiated = true)
}
}
}
Expand Down
Loading