Skip to content

Commit ecb5cc6

Browse files
xgouchet0xnm
xgouchet
authored and
0xnm
committed
RUM-6171 use FileObserver to limit calls to filesystem
1 parent dc4ab79 commit ecb5cc6

File tree

3 files changed

+89
-27
lines changed

3 files changed

+89
-27
lines changed

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

Lines changed: 43 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_FROM
13+
import android.os.FileObserver.MOVED_TO
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,35 @@ 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+
MOVED_FROM, DELETE -> {
73+
synchronized(knownFiles) {
74+
knownFiles.remove(file)
75+
}
76+
}
77+
}
78+
}
79+
}
80+
}
81+
82+
init {
83+
fileObserver.startWatching()
84+
}
85+
5586
// region FileOrchestrator
5687

5788
@WorkerThread
@@ -323,7 +354,9 @@ internal class BatchFileOrchestrator(
323354
}
324355

325356
private fun listBatchFiles(): List<File> {
326-
return rootDir.listFilesSafe(fileFilter, internalLogger).orEmpty().toList()
357+
return synchronized(knownFiles) {
358+
knownFiles.toList()
359+
}
327360
}
328361

329362
private fun listSortedBatchFiles(): List<File> {
@@ -340,28 +373,20 @@ internal class BatchFileOrchestrator(
340373
get() = File("${this.path}_metadata")
341374

342375
private val File.isBatchFile: Boolean
343-
get() = name.toLongOrNull() != null
376+
get() = name.isBatchFileName
377+
378+
private val String.isBatchFileName: Boolean
379+
get() = toLongOrNull() != null
344380

345381
private val List<File>.latestBatchFile: File?
346382
get() = maxOrNull()
347383

348384
// endregion
349385

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-
363386
companion object {
364387

388+
private const val FILE_OBSERVER_MASK = CREATE or DELETE or MOVED_TO or MOVED_FROM
389+
365390
const val DECREASE_PERCENT = 0.95
366391
const val INCREASE_PERCENT = 1.05
367392

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

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@
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
1213
import com.datadog.android.core.internal.metrics.RemovalReason
13-
import com.datadog.android.core.internal.persistence.file.FileOrchestrator
1414
import com.datadog.android.core.internal.persistence.file.FilePersistenceConfig
1515
import com.datadog.android.utils.forge.Configurator
1616
import com.datadog.android.utils.verifyLog
@@ -57,7 +57,7 @@ import java.util.concurrent.atomic.AtomicInteger
5757
@MockitoSettings(strictness = Strictness.LENIENT)
5858
internal class BatchFileOrchestratorTest {
5959

60-
private lateinit var testedOrchestrator: FileOrchestrator
60+
private lateinit var testedOrchestrator: BatchFileOrchestrator
6161

6262
@TempDir
6363
lateinit var tempDir: File
@@ -156,7 +156,11 @@ internal class BatchFileOrchestratorTest {
156156
var previousFile: File? = null
157157
val startTimestamp = System.currentTimeMillis()
158158
repeat(iterations) {
159-
previousFile = testedOrchestrator.getWritableFile(false)
159+
val file = testedOrchestrator.getWritableFile(false)
160+
if (file != previousFile) {
161+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, file?.name)
162+
}
163+
previousFile = file
160164
previousFile?.writeText(data)
161165
}
162166
val endTimestamp = System.currentTimeMillis()
@@ -291,11 +295,14 @@ internal class BatchFileOrchestratorTest {
291295
val oldTimestamp = System.currentTimeMillis() - oldFileAge
292296
val oldFile = File(fakeRootDir, oldTimestamp.toString())
293297
oldFile.createNewFile()
298+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFile.name)
294299
val oldFileMeta = File("${oldFile.path}_metadata")
295300
oldFileMeta.createNewFile()
301+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFileMeta.name)
296302
val youngTimestamp = System.currentTimeMillis() - RECENT_DELAY_MS - 1
297303
val youngFile = File(fakeRootDir, youngTimestamp.toString())
298304
youngFile.createNewFile()
305+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, youngFile.name)
299306

300307
// When
301308
val start = System.currentTimeMillis()
@@ -331,11 +338,14 @@ internal class BatchFileOrchestratorTest {
331338
val oldTimestamp = System.currentTimeMillis() - oldFileAge
332339
val oldFile = File(fakeRootDir, oldTimestamp.toString())
333340
oldFile.createNewFile()
341+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFile.name)
334342
val oldFileMeta = File("${oldFile.path}_metadata")
335343
oldFileMeta.createNewFile()
344+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFileMeta.name)
336345
val youngTimestamp = System.currentTimeMillis() - RECENT_DELAY_MS - 1
337346
val youngFile = File(fakeRootDir, youngTimestamp.toString())
338347
youngFile.createNewFile()
348+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, youngFile.name)
339349

340350
// When
341351
val start = System.currentTimeMillis()
@@ -345,6 +355,7 @@ internal class BatchFileOrchestratorTest {
345355
// cleanup shouldn't be performed during the next getWritableFile call
346356
val evenOlderFile = File(fakeRootDir, (oldTimestamp - 1).toString())
347357
evenOlderFile.createNewFile()
358+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, evenOlderFile.name)
348359
testedOrchestrator.getWritableFile(forceNewFile)
349360

350361
// Then
@@ -376,8 +387,10 @@ internal class BatchFileOrchestratorTest {
376387
val oldTimestamp = System.currentTimeMillis() - oldFileAge
377388
val oldFile = File(fakeRootDir, oldTimestamp.toString())
378389
oldFile.createNewFile()
390+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFile.name)
379391
val oldFileMeta = File("${oldFile.path}_metadata")
380392
oldFileMeta.createNewFile()
393+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFileMeta.name)
381394

382395
// When
383396
val start = System.currentTimeMillis()
@@ -386,6 +399,7 @@ internal class BatchFileOrchestratorTest {
386399
Thread.sleep(CLEANUP_FREQUENCY_THRESHOLD_MS + 1)
387400
val evenOlderFile = File(fakeRootDir, (oldTimestamp - 1).toString())
388401
evenOlderFile.createNewFile()
402+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, evenOlderFile.name)
389403
testedOrchestrator.getWritableFile(forceNewFile)
390404

391405
// Then
@@ -450,6 +464,7 @@ internal class BatchFileOrchestratorTest {
450464
assumeTrue(fakeRootDir.listFiles().isNullOrEmpty())
451465
val previousFile = testedOrchestrator.getWritableFile()
452466
checkNotNull(previousFile)
467+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, previousFile.name)
453468
previousFile.writeText(previousData)
454469
Thread.sleep(1)
455470

@@ -667,12 +682,16 @@ internal class BatchFileOrchestratorTest {
667682
// Given
668683
assumeTrue(fakeRootDir.listFiles().isNullOrEmpty())
669684
val filesCount = MAX_DISK_SPACE / MAX_BATCH_SIZE
670-
val files = (0..filesCount).map {
685+
val files = mutableListOf<File>()
686+
repeat(filesCount + 1) {
671687
val file = testedOrchestrator.getWritableFile()
672688
checkNotNull(file)
689+
if (files.none { it.name == file.name }) {
690+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, file.name)
691+
}
673692
file.writeText(previousData)
693+
files.add(file)
674694
Thread.sleep(1)
675-
file
676695
}
677696

678697
// When
@@ -831,11 +850,14 @@ internal class BatchFileOrchestratorTest {
831850
val oldTimestamp = System.currentTimeMillis() - oldFileAge
832851
val oldFile = File(fakeRootDir, oldTimestamp.toString())
833852
oldFile.createNewFile()
853+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFile.name)
834854
val oldFileMeta = File("${oldFile.path}_metadata")
835855
oldFileMeta.createNewFile()
856+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFileMeta.name)
836857
val youngTimestamp = System.currentTimeMillis() - RECENT_DELAY_MS - 1
837858
val youngFile = File(fakeRootDir, youngTimestamp.toString())
838859
youngFile.createNewFile()
860+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, youngFile.name)
839861

840862
// When
841863
val result = testedOrchestrator.getReadableFile(emptySet())
@@ -878,6 +900,7 @@ internal class BatchFileOrchestratorTest {
878900
val timestamp = System.currentTimeMillis() - (RECENT_DELAY_MS * 2)
879901
val file = File(fakeRootDir, timestamp.toString())
880902
file.createNewFile()
903+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, file.name)
881904

882905
// When
883906
val result = testedOrchestrator.getReadableFile(emptySet())
@@ -1039,10 +1062,16 @@ internal class BatchFileOrchestratorTest {
10391062
for (i in 1..count) {
10401063
// create both non readable and non writable files
10411064
expectedFiles.add(
1042-
File(fakeRootDir, (new + i).toString()).also { it.createNewFile() }
1065+
File(fakeRootDir, (new + i).toString()).also {
1066+
it.createNewFile()
1067+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, it.name)
1068+
}
10431069
)
10441070
expectedFiles.add(
1045-
File(fakeRootDir, (old - i).toString()).also { it.createNewFile() }
1071+
File(fakeRootDir, (old - i).toString()).also {
1072+
it.createNewFile()
1073+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, it.name)
1074+
}
10461075
)
10471076
}
10481077

@@ -1085,10 +1114,16 @@ internal class BatchFileOrchestratorTest {
10851114
for (i in 1..count) {
10861115
// create both non readable and non writable files
10871116
expectedFiles.add(
1088-
File(fakeRootDir, (new + i).toString()).also { it.createNewFile() }
1117+
File(fakeRootDir, (new + i).toString()).also {
1118+
it.createNewFile()
1119+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, it.name)
1120+
}
10891121
)
10901122
expectedFiles.add(
1091-
File(fakeRootDir, (old - i).toString()).also { it.createNewFile() }
1123+
File(fakeRootDir, (old - i).toString()).also {
1124+
it.createNewFile()
1125+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, it.name)
1126+
}
10921127
)
10931128
}
10941129

‎detekt_custom.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,7 @@ datadog:
391391
- "android.os.Bundle.get(kotlin.String?)"
392392
- "android.os.Bundle.getString(kotlin.String?)"
393393
- "android.os.Bundle.keySet()"
394+
- "android.os.FileObserver.startWatching()"
394395
- "android.os.Handler.constructor(android.os.Looper)"
395396
- "android.os.Handler.post(java.lang.Runnable)"
396397
- "android.os.Handler.postDelayed(java.lang.Runnable, kotlin.Long)"
@@ -874,6 +875,7 @@ datadog:
874875
- "kotlin.Array.orEmpty()"
875876
- "kotlin.Array.sorted()"
876877
- "kotlin.Array.toList()"
878+
- "kotlin.Array.toMutableSet()"
877879
- "kotlin.byteArrayOf(kotlin.ByteArray)"
878880
- "kotlin.ByteArray.any(kotlin.Function1)"
879881
- "kotlin.ByteArray.contentEquals(kotlin.ByteArray?)"

0 commit comments

Comments
 (0)