Skip to content

Commit 6d02b12

Browse files
xgouchetxgouchet
committed
RUM-6171 use FileObserver to limit calls to filesystem
1 parent dc4ab79 commit 6d02b12

File tree

2 files changed

+89
-26
lines changed

2 files changed

+89
-26
lines changed

‎dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileOrchestrator.kt

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66

77
package com.datadog.android.core.internal.persistence.file.batch
88

9+
import android.os.FileObserver
10+
import android.os.FileObserver.CREATE
11+
import android.os.FileObserver.DELETE
12+
import android.os.FileObserver.MOVED_TO
13+
import android.os.FileObserver.MOVED_FROM
914
import androidx.annotation.WorkerThread
1015
import com.datadog.android.api.InternalLogger
1116
import com.datadog.android.core.internal.metrics.BatchClosedMetadata
@@ -20,7 +25,6 @@ import com.datadog.android.core.internal.persistence.file.lengthSafe
2025
import com.datadog.android.core.internal.persistence.file.listFilesSafe
2126
import com.datadog.android.core.internal.persistence.file.mkdirsSafe
2227
import java.io.File
23-
import java.io.FileFilter
2428
import java.util.Locale
2529
import java.util.concurrent.atomic.AtomicInteger
2630
import kotlin.math.roundToLong
@@ -36,8 +40,6 @@ internal class BatchFileOrchestrator(
3640
private val pendingFiles: AtomicInteger = AtomicInteger(0)
3741
) : FileOrchestrator {
3842

39-
private val fileFilter = BatchFileFilter()
40-
4143
// Offset the recent threshold for read and write to avoid conflicts
4244
// Arbitrary offset as ±5% of the threshold
4345
@Suppress("UnsafeThirdPartyFunctionCall") // rounded Double isn't NaN
@@ -52,6 +54,37 @@ internal class BatchFileOrchestrator(
5254
private var lastFileAccessTimestamp: Long = 0L
5355
private var lastCleanupTimestamp: Long = 0L
5456

57+
private val knownFiles: MutableSet<File> =
58+
rootDir.listFilesSafe(internalLogger)?.toMutableSet() ?: mutableSetOf()
59+
60+
@Suppress("DEPRECATION") // Recommended constructor only available in API 29 Q
61+
internal val fileObserver = object : FileObserver(rootDir.path, FILE_OBSERVER_MASK) {
62+
override fun onEvent(event: Int, name: String?) {
63+
if (!name.isNullOrEmpty() && name.isBatchFileName) {
64+
val file = File(rootDir, name)
65+
when (event) {
66+
MOVED_TO, CREATE -> {
67+
synchronized(knownFiles) {
68+
knownFiles.add(file)
69+
}
70+
71+
}
72+
73+
MOVED_FROM, DELETE -> {
74+
synchronized(knownFiles) {
75+
knownFiles.remove(file)
76+
}
77+
}
78+
}
79+
}
80+
}
81+
82+
}
83+
84+
init {
85+
fileObserver.startWatching()
86+
}
87+
5588
// region FileOrchestrator
5689

5790
@WorkerThread
@@ -323,7 +356,9 @@ internal class BatchFileOrchestrator(
323356
}
324357

325358
private fun listBatchFiles(): List<File> {
326-
return rootDir.listFilesSafe(fileFilter, internalLogger).orEmpty().toList()
359+
return synchronized(knownFiles) {
360+
knownFiles.toList()
361+
}
327362
}
328363

329364
private fun listSortedBatchFiles(): List<File> {
@@ -340,28 +375,20 @@ internal class BatchFileOrchestrator(
340375
get() = File("${this.path}_metadata")
341376

342377
private val File.isBatchFile: Boolean
343-
get() = name.toLongOrNull() != null
378+
get() = name.isBatchFileName
379+
380+
private val String.isBatchFileName: Boolean
381+
get() = toLongOrNull() != null
344382

345383
private val List<File>.latestBatchFile: File?
346384
get() = maxOrNull()
347385

348386
// endregion
349387

350-
// region FileFilter
351-
352-
internal inner class BatchFileFilter : FileFilter {
353-
@Suppress("ReturnCount")
354-
override fun accept(file: File?): Boolean {
355-
if (file == null) return false
356-
357-
return file.isBatchFile
358-
}
359-
}
360-
361-
// endregion
362-
363388
companion object {
364389

390+
private const val FILE_OBSERVER_MASK = CREATE or DELETE or MOVED_TO or MOVED_FROM
391+
365392
const val DECREASE_PERCENT = 0.95
366393
const val INCREASE_PERCENT = 1.05
367394

‎dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileOrchestratorTest.kt

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
package com.datadog.android.core.internal.persistence.file.batch
88

9+
import android.os.FileObserver
910
import com.datadog.android.api.InternalLogger
1011
import com.datadog.android.core.internal.metrics.BatchClosedMetadata
1112
import com.datadog.android.core.internal.metrics.MetricsDispatcher
@@ -57,7 +58,7 @@ import java.util.concurrent.atomic.AtomicInteger
5758
@MockitoSettings(strictness = Strictness.LENIENT)
5859
internal class BatchFileOrchestratorTest {
5960

60-
private lateinit var testedOrchestrator: FileOrchestrator
61+
private lateinit var testedOrchestrator: BatchFileOrchestrator
6162

6263
@TempDir
6364
lateinit var tempDir: File
@@ -156,7 +157,11 @@ internal class BatchFileOrchestratorTest {
156157
var previousFile: File? = null
157158
val startTimestamp = System.currentTimeMillis()
158159
repeat(iterations) {
159-
previousFile = testedOrchestrator.getWritableFile(false)
160+
val file = testedOrchestrator.getWritableFile(false)
161+
if (file != previousFile) {
162+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, file?.name)
163+
}
164+
previousFile = file
160165
previousFile?.writeText(data)
161166
}
162167
val endTimestamp = System.currentTimeMillis()
@@ -291,11 +296,14 @@ internal class BatchFileOrchestratorTest {
291296
val oldTimestamp = System.currentTimeMillis() - oldFileAge
292297
val oldFile = File(fakeRootDir, oldTimestamp.toString())
293298
oldFile.createNewFile()
299+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFile.name)
294300
val oldFileMeta = File("${oldFile.path}_metadata")
295301
oldFileMeta.createNewFile()
302+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFileMeta.name)
296303
val youngTimestamp = System.currentTimeMillis() - RECENT_DELAY_MS - 1
297304
val youngFile = File(fakeRootDir, youngTimestamp.toString())
298305
youngFile.createNewFile()
306+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, youngFile.name)
299307

300308
// When
301309
val start = System.currentTimeMillis()
@@ -331,11 +339,14 @@ internal class BatchFileOrchestratorTest {
331339
val oldTimestamp = System.currentTimeMillis() - oldFileAge
332340
val oldFile = File(fakeRootDir, oldTimestamp.toString())
333341
oldFile.createNewFile()
342+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFile.name)
334343
val oldFileMeta = File("${oldFile.path}_metadata")
335344
oldFileMeta.createNewFile()
345+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFileMeta.name)
336346
val youngTimestamp = System.currentTimeMillis() - RECENT_DELAY_MS - 1
337347
val youngFile = File(fakeRootDir, youngTimestamp.toString())
338348
youngFile.createNewFile()
349+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, youngFile.name)
339350

340351
// When
341352
val start = System.currentTimeMillis()
@@ -345,6 +356,7 @@ internal class BatchFileOrchestratorTest {
345356
// cleanup shouldn't be performed during the next getWritableFile call
346357
val evenOlderFile = File(fakeRootDir, (oldTimestamp - 1).toString())
347358
evenOlderFile.createNewFile()
359+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, evenOlderFile.name)
348360
testedOrchestrator.getWritableFile(forceNewFile)
349361

350362
// Then
@@ -376,8 +388,10 @@ internal class BatchFileOrchestratorTest {
376388
val oldTimestamp = System.currentTimeMillis() - oldFileAge
377389
val oldFile = File(fakeRootDir, oldTimestamp.toString())
378390
oldFile.createNewFile()
391+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFile.name)
379392
val oldFileMeta = File("${oldFile.path}_metadata")
380393
oldFileMeta.createNewFile()
394+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFileMeta.name)
381395

382396
// When
383397
val start = System.currentTimeMillis()
@@ -386,6 +400,7 @@ internal class BatchFileOrchestratorTest {
386400
Thread.sleep(CLEANUP_FREQUENCY_THRESHOLD_MS + 1)
387401
val evenOlderFile = File(fakeRootDir, (oldTimestamp - 1).toString())
388402
evenOlderFile.createNewFile()
403+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, evenOlderFile.name)
389404
testedOrchestrator.getWritableFile(forceNewFile)
390405

391406
// Then
@@ -450,6 +465,7 @@ internal class BatchFileOrchestratorTest {
450465
assumeTrue(fakeRootDir.listFiles().isNullOrEmpty())
451466
val previousFile = testedOrchestrator.getWritableFile()
452467
checkNotNull(previousFile)
468+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, previousFile.name)
453469
previousFile.writeText(previousData)
454470
Thread.sleep(1)
455471

@@ -667,12 +683,16 @@ internal class BatchFileOrchestratorTest {
667683
// Given
668684
assumeTrue(fakeRootDir.listFiles().isNullOrEmpty())
669685
val filesCount = MAX_DISK_SPACE / MAX_BATCH_SIZE
670-
val files = (0..filesCount).map {
686+
val files = mutableListOf<File>()
687+
repeat(filesCount + 1) {
671688
val file = testedOrchestrator.getWritableFile()
672689
checkNotNull(file)
690+
if (files.none { it.name == file.name }) {
691+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, file.name)
692+
}
673693
file.writeText(previousData)
694+
files.add(file)
674695
Thread.sleep(1)
675-
file
676696
}
677697

678698
// When
@@ -831,11 +851,14 @@ internal class BatchFileOrchestratorTest {
831851
val oldTimestamp = System.currentTimeMillis() - oldFileAge
832852
val oldFile = File(fakeRootDir, oldTimestamp.toString())
833853
oldFile.createNewFile()
854+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFile.name)
834855
val oldFileMeta = File("${oldFile.path}_metadata")
835856
oldFileMeta.createNewFile()
857+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFileMeta.name)
836858
val youngTimestamp = System.currentTimeMillis() - RECENT_DELAY_MS - 1
837859
val youngFile = File(fakeRootDir, youngTimestamp.toString())
838860
youngFile.createNewFile()
861+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, youngFile.name)
839862

840863
// When
841864
val result = testedOrchestrator.getReadableFile(emptySet())
@@ -878,6 +901,7 @@ internal class BatchFileOrchestratorTest {
878901
val timestamp = System.currentTimeMillis() - (RECENT_DELAY_MS * 2)
879902
val file = File(fakeRootDir, timestamp.toString())
880903
file.createNewFile()
904+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, file.name)
881905

882906
// When
883907
val result = testedOrchestrator.getReadableFile(emptySet())
@@ -1039,10 +1063,16 @@ internal class BatchFileOrchestratorTest {
10391063
for (i in 1..count) {
10401064
// create both non readable and non writable files
10411065
expectedFiles.add(
1042-
File(fakeRootDir, (new + i).toString()).also { it.createNewFile() }
1066+
File(fakeRootDir, (new + i).toString()).also {
1067+
it.createNewFile()
1068+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, it.name)
1069+
}
10431070
)
10441071
expectedFiles.add(
1045-
File(fakeRootDir, (old - i).toString()).also { it.createNewFile() }
1072+
File(fakeRootDir, (old - i).toString()).also {
1073+
it.createNewFile()
1074+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, it.name)
1075+
}
10461076
)
10471077
}
10481078

@@ -1085,10 +1115,16 @@ internal class BatchFileOrchestratorTest {
10851115
for (i in 1..count) {
10861116
// create both non readable and non writable files
10871117
expectedFiles.add(
1088-
File(fakeRootDir, (new + i).toString()).also { it.createNewFile() }
1118+
File(fakeRootDir, (new + i).toString()).also {
1119+
it.createNewFile()
1120+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, it.name)
1121+
}
10891122
)
10901123
expectedFiles.add(
1091-
File(fakeRootDir, (old - i).toString()).also { it.createNewFile() }
1124+
File(fakeRootDir, (old - i).toString()).also {
1125+
it.createNewFile()
1126+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, it.name)
1127+
}
10921128
)
10931129
}
10941130

0 commit comments

Comments
 (0)