RUM-6171 use FileObserver to limit calls to filesystem #2718
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,11 @@ | |
|
|
||
| package com.datadog.android.core.internal.persistence.file.batch | ||
|
|
||
| import android.os.FileObserver | ||
| import android.os.FileObserver.CREATE | ||
| import android.os.FileObserver.DELETE | ||
| import android.os.FileObserver.MOVED_TO | ||
| import android.os.FileObserver.MOVED_FROM | ||
| import androidx.annotation.WorkerThread | ||
| import com.datadog.android.api.InternalLogger | ||
| import com.datadog.android.core.internal.metrics.BatchClosedMetadata | ||
|
|
@@ -20,7 +25,6 @@ import com.datadog.android.core.internal.persistence.file.lengthSafe | |
| import com.datadog.android.core.internal.persistence.file.listFilesSafe | ||
| import com.datadog.android.core.internal.persistence.file.mkdirsSafe | ||
| import java.io.File | ||
| import java.util.Locale | ||
| import java.util.concurrent.atomic.AtomicInteger | ||
| import kotlin.math.roundToLong | ||
|
|
@@ -36,8 +40,6 @@ internal class BatchFileOrchestrator( | |
| private val pendingFiles: AtomicInteger = AtomicInteger(0) | ||
| ) : FileOrchestrator { | ||
|
|
||
| // Offset the recent threshold for read and write to avoid conflicts | ||
| // Arbitrary offset as ±5% of the threshold | ||
| @Suppress("UnsafeThirdPartyFunctionCall") // rounded Double isn't NaN | ||
|
|
@@ -52,6 +54,37 @@ internal class BatchFileOrchestrator( | |
| private var lastFileAccessTimestamp: Long = 0L | ||
| private var lastCleanupTimestamp: Long = 0L | ||
|
|
||
| private val knownFiles: MutableSet<File> = | ||
| rootDir.listFilesSafe(internalLogger)?.toMutableSet() ?: mutableSetOf() | ||
|
|
||
| @Suppress("DEPRECATION") // Recommended constructor only available in API 29 Q | ||
| internal val fileObserver = object : FileObserver(rootDir.path, FILE_OBSERVER_MASK) { | ||
| override fun onEvent(event: Int, name: String?) { | ||
| if (!name.isNullOrEmpty() && name.isBatchFileName) { | ||
| val file = File(rootDir, name) | ||
| when (event) { | ||
| MOVED_TO, CREATE -> { | ||
| synchronized(knownFiles) { | ||
| knownFiles.add(file) | ||
| } | ||
|
|
||
| } | ||
|
|
||
| MOVED_FROM, DELETE -> { | ||
| synchronized(knownFiles) { | ||
| knownFiles.remove(file) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| } | ||
|
|
||
| init { | ||
| fileObserver.startWatching() | ||
|
There was a problem hiding this comment. should we stop watching at some point ? not sure when though or if needed There was a problem hiding this comment. In theory yes but not sure when either There was a problem hiding this comment. We could use We probably need to still find a way to stop watching, because if There was a problem hiding this comment. Apparently we should be good, because there is |
||
| } | ||
|
|
||
| // region FileOrchestrator | ||
|
|
||
| @WorkerThread | ||
|
|
@@ -323,7 +356,9 @@ internal class BatchFileOrchestrator( | |
| } | ||
|
|
||
| private fun listBatchFiles(): List<File> { | ||
| return synchronized(knownFiles) { | ||
| knownFiles.toList() | ||
| } | ||
| } | ||
|
|
||
| private fun listSortedBatchFiles(): List<File> { | ||
|
|
@@ -340,28 +375,20 @@ internal class BatchFileOrchestrator( | |
| get() = File("${this.path}_metadata") | ||
|
|
||
| private val File.isBatchFile: Boolean | ||
| get() = name.isBatchFileName | ||
|
|
||
| private val String.isBatchFileName: Boolean | ||
| get() = toLongOrNull() != null | ||
|
|
||
| private val List<File>.latestBatchFile: File? | ||
| get() = maxOrNull() | ||
|
|
||
| // endregion | ||
|
|
||
| companion object { | ||
|
|
||
| private const val FILE_OBSERVER_MASK = CREATE or DELETE or MOVED_TO or MOVED_FROM | ||
|
|
||
| const val DECREASE_PERCENT = 0.95 | ||
| const val INCREASE_PERCENT = 1.05 | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realized the it may bring the following concern: this will be called on the user thread during SDK initialization and since it is I/O call, it may slow down SDK start (especially if there is a lot of files on the disk).
Instead, we maybe can to initial read in the
getWritableFile/getReadableFilemethods (well, on any initial access of this property), this will shift it to the worker thread improving SDK initialization.Same may apply to the
rootfolder creation.