Skip to content

Commit 5c82f6f

Browse files
xgouchetxgouchet
committed
RUM-6171 use FileObserver to limit calls to filesystem
1 parent d7410ff commit 5c82f6f

File tree

2 files changed

+79
-9
lines changed

2 files changed

+79
-9
lines changed

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

Lines changed: 35 additions & 1 deletion
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 androidx.annotation.WorkerThread
1011
import com.datadog.android.api.InternalLogger
1112
import com.datadog.android.core.internal.metrics.BatchClosedMetadata
@@ -52,6 +53,37 @@ internal class BatchFileOrchestrator(
5253
private var lastFileAccessTimestamp: Long = 0L
5354
private var lastCleanupTimestamp: Long = 0L
5455

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

5789
@WorkerThread
@@ -323,7 +355,9 @@ internal class BatchFileOrchestrator(
323355
}
324356

325357
private fun listBatchFiles(): List<File> {
326-
return rootDir.listFilesSafe(fileFilter, internalLogger).orEmpty().toList()
358+
return synchronized(knownFiles) {
359+
knownFiles.toList()
360+
}
327361
}
328362

329363
private fun listSortedBatchFiles(): List<File> {

‎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)