Skip to content

Commit c260956

Browse files
xgouchetkikoveiga
xgouchet
authored and
kikoveiga
committed
RUM-6171 use FileObserver to limit calls to filesystem
1 parent 3e289ad commit c260956

File tree

3 files changed

+84
-27
lines changed

3 files changed

+84
-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
@@ -318,7 +349,9 @@ internal class BatchFileOrchestrator(
318349
}
319350

320351
private fun listBatchFiles(): List<File> {
321-
return rootDir.listFilesSafe(fileFilter, internalLogger).orEmpty().toList()
352+
return synchronized(knownFiles) {
353+
knownFiles.toList()
354+
}
322355
}
323356

324357
private fun listSortedBatchFiles(): List<File> {
@@ -335,28 +368,20 @@ internal class BatchFileOrchestrator(
335368
get() = File("${this.path}_metadata")
336369

337370
private val File.isBatchFile: Boolean
338-
get() = name.toLongOrNull() != null
371+
get() = name.isBatchFileName
372+
373+
private val String.isBatchFileName: Boolean
374+
get() = toLongOrNull() != null
339375

340376
private val List<File>.latestBatchFile: File?
341377
get() = maxOrNull()
342378

343379
// endregion
344380

345-
// region FileFilter
346-
347-
internal inner class BatchFileFilter : FileFilter {
348-
@Suppress("ReturnCount")
349-
override fun accept(file: File?): Boolean {
350-
if (file == null) return false
351-
352-
return file.isBatchFile
353-
}
354-
}
355-
356-
// endregion
357-
358381
companion object {
359382

383+
private const val FILE_OBSERVER_MASK = CREATE or DELETE or MOVED_TO or MOVED_FROM
384+
360385
const val DECREASE_PERCENT = 0.95
361386
const val INCREASE_PERCENT = 1.05
362387

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

Lines changed: 39 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
@@ -35,7 +35,6 @@ import org.mockito.kotlin.argumentCaptor
3535
import org.mockito.kotlin.doReturn
3636
import org.mockito.kotlin.eq
3737
import org.mockito.kotlin.mock
38-
import org.mockito.kotlin.times
3938
import org.mockito.kotlin.verify
4039
import org.mockito.kotlin.verifyNoInteractions
4140
import org.mockito.kotlin.verifyNoMoreInteractions
@@ -55,7 +54,7 @@ import java.util.concurrent.atomic.AtomicInteger
5554
@MockitoSettings(strictness = Strictness.LENIENT)
5655
internal class BatchFileOrchestratorTest {
5756

58-
private lateinit var testedOrchestrator: FileOrchestrator
57+
private lateinit var testedOrchestrator: BatchFileOrchestrator
5958

6059
@TempDir
6160
lateinit var tempDir: File
@@ -239,11 +238,14 @@ internal class BatchFileOrchestratorTest {
239238
val oldTimestamp = System.currentTimeMillis() - oldFileAge
240239
val oldFile = File(fakeRootDir, oldTimestamp.toString())
241240
oldFile.createNewFile()
241+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFile.name)
242242
val oldFileMeta = File("${oldFile.path}_metadata")
243243
oldFileMeta.createNewFile()
244+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFileMeta.name)
244245
val youngTimestamp = System.currentTimeMillis() - RECENT_DELAY_MS - 1
245246
val youngFile = File(fakeRootDir, youngTimestamp.toString())
246247
youngFile.createNewFile()
248+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, youngFile.name)
247249

248250
// When
249251
val start = System.currentTimeMillis()
@@ -277,11 +279,14 @@ internal class BatchFileOrchestratorTest {
277279
val oldTimestamp = System.currentTimeMillis() - oldFileAge
278280
val oldFile = File(fakeRootDir, oldTimestamp.toString())
279281
oldFile.createNewFile()
282+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFile.name)
280283
val oldFileMeta = File("${oldFile.path}_metadata")
281284
oldFileMeta.createNewFile()
285+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFileMeta.name)
282286
val youngTimestamp = System.currentTimeMillis() - RECENT_DELAY_MS - 1
283287
val youngFile = File(fakeRootDir, youngTimestamp.toString())
284288
youngFile.createNewFile()
289+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, youngFile.name)
285290

286291
// When
287292
val start = System.currentTimeMillis()
@@ -291,6 +296,7 @@ internal class BatchFileOrchestratorTest {
291296
// cleanup shouldn't be performed during the next getWritableFile call
292297
val evenOlderFile = File(fakeRootDir, (oldTimestamp - 1).toString())
293298
evenOlderFile.createNewFile()
299+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, evenOlderFile.name)
294300
testedOrchestrator.getWritableFile()
295301

296302
// Then
@@ -320,8 +326,10 @@ internal class BatchFileOrchestratorTest {
320326
val oldTimestamp = System.currentTimeMillis() - oldFileAge
321327
val oldFile = File(fakeRootDir, oldTimestamp.toString())
322328
oldFile.createNewFile()
329+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFile.name)
323330
val oldFileMeta = File("${oldFile.path}_metadata")
324331
oldFileMeta.createNewFile()
332+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFileMeta.name)
325333

326334
// When
327335
val start = System.currentTimeMillis()
@@ -330,6 +338,7 @@ internal class BatchFileOrchestratorTest {
330338
Thread.sleep(CLEANUP_FREQUENCY_THRESHOLD_MS + 1)
331339
val evenOlderFile = File(fakeRootDir, (oldTimestamp - 1).toString())
332340
evenOlderFile.createNewFile()
341+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, evenOlderFile.name)
333342
testedOrchestrator.getWritableFile()
334343

335344
// Then
@@ -392,6 +401,7 @@ internal class BatchFileOrchestratorTest {
392401
assumeTrue(fakeRootDir.listFiles().isNullOrEmpty())
393402
val previousFile = testedOrchestrator.getWritableFile()
394403
checkNotNull(previousFile)
404+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, previousFile.name)
395405
previousFile.writeText(previousData)
396406
Thread.sleep(1)
397407

@@ -594,12 +604,16 @@ internal class BatchFileOrchestratorTest {
594604
// Given
595605
assumeTrue(fakeRootDir.listFiles().isNullOrEmpty())
596606
val filesCount = MAX_DISK_SPACE / MAX_BATCH_SIZE
597-
val files = (0..filesCount).map {
607+
val files = mutableListOf<File>()
608+
repeat(filesCount + 1) {
598609
val file = testedOrchestrator.getWritableFile()
599610
checkNotNull(file)
611+
if (files.none { it.name == file.name }) {
612+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, file.name)
613+
}
600614
file.writeText(previousData)
615+
files.add(file)
601616
Thread.sleep(1)
602-
file
603617
}
604618

605619
// When
@@ -720,11 +734,14 @@ internal class BatchFileOrchestratorTest {
720734
val oldTimestamp = System.currentTimeMillis() - oldFileAge
721735
val oldFile = File(fakeRootDir, oldTimestamp.toString())
722736
oldFile.createNewFile()
737+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFile.name)
723738
val oldFileMeta = File("${oldFile.path}_metadata")
724739
oldFileMeta.createNewFile()
740+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, oldFileMeta.name)
725741
val youngTimestamp = System.currentTimeMillis() - RECENT_DELAY_MS - 1
726742
val youngFile = File(fakeRootDir, youngTimestamp.toString())
727743
youngFile.createNewFile()
744+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, youngFile.name)
728745

729746
// When
730747
val result = testedOrchestrator.getReadableFile(emptySet())
@@ -767,6 +784,7 @@ internal class BatchFileOrchestratorTest {
767784
val timestamp = System.currentTimeMillis() - (RECENT_DELAY_MS * 2)
768785
val file = File(fakeRootDir, timestamp.toString())
769786
file.createNewFile()
787+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, file.name)
770788

771789
// When
772790
val result = testedOrchestrator.getReadableFile(emptySet())
@@ -928,10 +946,16 @@ internal class BatchFileOrchestratorTest {
928946
for (i in 1..count) {
929947
// create both non readable and non writable files
930948
expectedFiles.add(
931-
File(fakeRootDir, (new + i).toString()).also { it.createNewFile() }
949+
File(fakeRootDir, (new + i).toString()).also {
950+
it.createNewFile()
951+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, it.name)
952+
}
932953
)
933954
expectedFiles.add(
934-
File(fakeRootDir, (old - i).toString()).also { it.createNewFile() }
955+
File(fakeRootDir, (old - i).toString()).also {
956+
it.createNewFile()
957+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, it.name)
958+
}
935959
)
936960
}
937961

@@ -974,10 +998,16 @@ internal class BatchFileOrchestratorTest {
974998
for (i in 1..count) {
975999
// create both non readable and non writable files
9761000
expectedFiles.add(
977-
File(fakeRootDir, (new + i).toString()).also { it.createNewFile() }
1001+
File(fakeRootDir, (new + i).toString()).also {
1002+
it.createNewFile()
1003+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, it.name)
1004+
}
9781005
)
9791006
expectedFiles.add(
980-
File(fakeRootDir, (old - i).toString()).also { it.createNewFile() }
1007+
File(fakeRootDir, (old - i).toString()).also {
1008+
it.createNewFile()
1009+
testedOrchestrator.fileObserver.onEvent(FileObserver.CREATE, it.name)
1010+
}
9811011
)
9821012
}
9831013

‎detekt_custom_safe_calls.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ datadog:
4646
- "android.os.Bundle.get(kotlin.String?)"
4747
- "android.os.Bundle.getString(kotlin.String?)"
4848
- "android.os.Bundle.keySet()"
49+
- "android.os.FileObserver.startWatching()"
4950
- "android.os.Handler.constructor(android.os.Looper)"
5051
- "android.os.Handler.post(java.lang.Runnable)"
5152
- "android.os.Handler.postDelayed(java.lang.Runnable, kotlin.Long)"
@@ -665,6 +666,7 @@ datadog:
665666
- "kotlin.Array.orEmpty()"
666667
- "kotlin.Array.sorted()"
667668
- "kotlin.Array.toList()"
669+
- "kotlin.Array.toMutableSet()"
668670
- "kotlin.byteArrayOf(kotlin.ByteArray)"
669671
- "kotlin.ByteArray.any(kotlin.Function1)"
670672
- "kotlin.ByteArray.contentEquals(kotlin.ByteArray?)"

0 commit comments

Comments
 (0)