removed dummyTaskRecord.
authorJerome Dochez <jedo@google.com>
Thu, 1 Aug 2019 20:31:46 +0000 (13:31 -0700)
committerJerome Dochez <jedo@google.com>
Fri, 2 Aug 2019 17:14:56 +0000 (17:14 +0000)
it prevented classloaders to be gc'ed when users switched
classpath (by adding/removing a plugin for instance).

Test: Adapted existing.
Fixes: 138649175
Change-Id: I364d2accd8773879560a99f626900f67b656c87f
(cherry picked from commit 9fe43fca763d0a05f4baa59147c1f9ff2d707633)

build-system/gradle-core/src/main/java/com/android/build/gradle/internal/profile/TaskProfilingRecord.kt
build-system/gradle-core/src/main/java/com/android/build/gradle/internal/tasks/Workers.kt
build-system/gradle-core/src/test/java/com/android/build/gradle/internal/profile/TaskProfilingRecordTest.kt
build-system/gradle-core/src/test/java/com/android/build/gradle/internal/profile/WorkerProfilingRecordTest.kt

index 0e9fc7bfb6b5336a0570a694414be1b379fde859..761c2024cff49951adfd3078b11aa332994fd5a9 100644 (file)
@@ -32,13 +32,19 @@ import javax.annotation.concurrent.GuardedBy
  * Information contained in each instance will be use to upload our performance data per task
  * once optional workers profiling information has been gathered.
  */
-open class TaskProfilingRecord {
-
-    private val taskPath: String
-    internal val projectPath: String
+open class TaskProfilingRecord
+/**
+ * Construct a new task profiling record with the [GradleBuildProfileSpan] and decorations like
+ * project path and variant name.
+ */(
+    private val recordWriter: ProfileRecordWriter,
+    span: GradleBuildProfileSpan.Builder,
+    private val taskPath: String,
+    internal val projectPath: String,
     internal val variant: String?
-    private val recordWriter: ProfileRecordWriter
-    val spanBuilder: GradleBuildProfileSpan.Builder
+) {
+
+    val spanBuilder: GradleBuildProfileSpan.Builder = span
     private val workerRecordList: MutableMap<String, WorkerProfilingRecord> = mutableMapOf()
     private val startTime = clock.instant()
     private var endTime = Instant.MIN
@@ -84,24 +90,6 @@ open class TaskProfilingRecord {
         SPAN_CLOSED
     }
 
-    /**
-     * Construct a new task profiling record with the [GradleBuildProfileSpan] and decorations like
-     * project path and variant name.
-     */
-    constructor(
-        recordWriter: ProfileRecordWriter,
-        span: GradleBuildProfileSpan.Builder,
-        taskPath: String,
-        projectPath: String,
-        variant: String?
-    ) {
-        this.taskPath = taskPath
-        this.recordWriter = recordWriter
-        this.projectPath = projectPath
-        this.spanBuilder = span
-        this.variant = variant
-    }
-
     fun setTaskWaiting() {
         status.set(Status.AWAIT)
     }
@@ -134,8 +122,7 @@ open class TaskProfilingRecord {
     }
 
     @Synchronized
-    open fun get(key: String): WorkerProfilingRecord = workerRecordList[key]
-        ?: dummyTaskRecord.get(key)
+    open fun get(key: String): WorkerProfilingRecord? = workerRecordList[key]
 
     @Synchronized
     fun allWorkersFinished(): Boolean {
@@ -212,36 +199,11 @@ open class TaskProfilingRecord {
         taskSpans.add(builder.build())
     }
 
-    private constructor() {
-        this.recordWriter = ProcessProfileWriter.get()
-        this.taskPath = "dummy"
-        this.projectPath = ":dummy"
-        this.variant = "dummy"
-        this.spanBuilder = GradleBuildProfileSpan.newBuilder()
-    }
-
     companion object {
         /**
          * Clock used to measure tasks and workers timings.
          */
         @VisibleForTesting
         var clock: Clock = Clock.systemDefaultZone()
-
-        /**
-         * Singleton object to satisfy usages when [ProfilerInitializer.recordingBuildListener]
-         * does not exist.
-         */
-        val dummyTaskRecord: TaskProfilingRecord = object : TaskProfilingRecord() {
-            override fun addWorker(key: String, type: GradleBuildProfileSpan.ExecutionType) {}
-            override fun get(key: String): WorkerProfilingRecord {
-                val workerProfilingRecord = WorkerProfilingRecord(
-                    "dummy",
-                    GradleBuildProfileSpan.ExecutionType.WORKER_EXECUTION,
-                    clock.instant()
-                )
-                workerProfilingRecord.executionStarted()
-                return workerProfilingRecord
-            }
-        }
     }
 }
index 487cc9562881293323a2999a89283ba77b0a24a4..1dae46cd7bd9231969e0c4f4156be63e51e90095 100644 (file)
@@ -18,7 +18,6 @@ package com.android.build.gradle.internal.tasks
 
 import com.android.annotations.concurrency.GuardedBy
 import com.android.build.gradle.internal.profile.ProfilerInitializer
-import com.android.build.gradle.internal.profile.TaskProfilingRecord
 import com.android.build.gradle.options.BooleanOption
 import com.android.build.gradle.options.IntegerOption
 import com.android.build.gradle.options.ProjectOptions
@@ -105,11 +104,11 @@ object Workers {
      * @return an instance of [WorkerExecutorFacade].
      */
     fun preferThreads(projectName: String, owner: String, workerExecutor: WorkerExecutor): WorkerExecutorFacade {
-        return Workers.ProfileAwareExecutorServiceAdapter(
+        return ProfileAwareExecutorServiceAdapter(
             projectName,
             owner,
             defaultExecutor,
-            Workers.preferWorkers(projectName, owner, workerExecutor))
+            preferWorkers(projectName, owner, workerExecutor))
     }
 
     /**
@@ -123,7 +122,7 @@ object Workers {
      * @return an instance of [WorkerExecutorFacade]
      */
     fun withThreads(projectName: String, owner: String)=
-        Workers.ProfileAwareExecutorServiceAdapter(projectName, owner, defaultExecutor)
+        ProfileAwareExecutorServiceAdapter(projectName, owner, defaultExecutor)
 
     private const val MAX_AAPT2_THREAD_POOL_SIZE = 8
 
@@ -234,8 +233,7 @@ object Workers {
         WorkerExecutorFacade {
 
         val taskRecord by lazy {
-            (ProfilerInitializer.getListener()?.getTaskRecord(owner)
-                ?: TaskProfilingRecord.dummyTaskRecord)
+            ProfilerInitializer.getListener()?.getTaskRecord(owner)
         }
 
         override fun submit(
@@ -265,7 +263,7 @@ object Workers {
                 workerKey
             )
 
-            taskRecord.addWorker(workerKey, GradleBuildProfileSpan.ExecutionType.WORKER_EXECUTION)
+            taskRecord?.addWorker(workerKey, GradleBuildProfileSpan.ExecutionType.WORKER_EXECUTION)
 
             val classpath = configuration.classPath.toList()
 
@@ -280,7 +278,7 @@ object Workers {
 
         override fun await() {
             try {
-                taskRecord.setTaskWaiting()
+                taskRecord?.setTaskWaiting()
                 workerExecutor.await()
             } catch (e: WorkerExecutionException) {
                 throw WorkerExecutorException(e.causes)
@@ -303,7 +301,7 @@ object Workers {
          * @TaskAction starts (so, we are safe!).
          */
         override fun close() {
-            taskRecord.setTaskClosed()
+            taskRecord?.setTaskClosed()
         }
     }
 
@@ -366,13 +364,12 @@ object Workers {
         ExecutorServiceAdapter(projectName, owner, executor, delegate) {
 
         private val taskRecord by lazy {
-            (ProfilerInitializer.getListener()?.getTaskRecord(owner)
-                ?: TaskProfilingRecord.dummyTaskRecord)
+            ProfilerInitializer.getListener()?.getTaskRecord(owner)
         }
 
         override fun workerSubmission(workerKey: String) {
             super.workerSubmission(workerKey)
-            taskRecord.addWorker(workerKey, GradleBuildProfileSpan.ExecutionType.THREAD_EXECUTION)
+            taskRecord?.addWorker(workerKey, GradleBuildProfileSpan.ExecutionType.THREAD_EXECUTION)
         }
     }
 }
\ No newline at end of file
index 87018577899fd328c737c7983c3bd9435c16ad3e..a6990546fb3d253c0f87572d8b04de0360a016b6 100644 (file)
@@ -46,14 +46,14 @@ class TaskProfilingRecordTest {
         testTaskRecord.addWorker("third")
 
         assertThat(testTaskRecord.allWorkersFinished()).isFalse()
-        testTaskRecord.get("first").executionStarted()
-        testTaskRecord.get("first").executionFinished()
+        testTaskRecord.get("first")?.executionStarted()
+        testTaskRecord.get("first")?.executionFinished()
         assertThat(testTaskRecord.allWorkersFinished()).isFalse()
-        testTaskRecord.get("second").executionStarted()
-        testTaskRecord.get("second").executionFinished()
+        testTaskRecord.get("second")?.executionStarted()
+        testTaskRecord.get("second")?.executionFinished()
         assertThat(testTaskRecord.allWorkersFinished()).isFalse()
-        testTaskRecord.get("third").executionStarted()
-        testTaskRecord.get("third").executionFinished()
+        testTaskRecord.get("third")?.executionStarted()
+        testTaskRecord.get("third")?.executionFinished()
         assertThat(testTaskRecord.allWorkersFinished()).isTrue()
     }
 
@@ -63,11 +63,11 @@ class TaskProfilingRecordTest {
         assertThat(testTaskRecord.allWorkersFinished()).isFalse()
 
         resetClockTo(300)
-        testTaskRecord.get("first").executionStarted()
+        testTaskRecord.get("first")?.executionStarted()
         assertThat(testTaskRecord.allWorkersFinished()).isFalse()
 
         resetClockTo(350)
-        testTaskRecord.get("first").executionFinished()
+        testTaskRecord.get("first")?.executionFinished()
         assertThat(testTaskRecord.minimumWaitTime()).isEqualTo(Duration.ofMillis(200))
         assertThat(testTaskRecord.duration()).isEqualTo(Duration.ofMillis(250))
     }
@@ -82,22 +82,22 @@ class TaskProfilingRecordTest {
         assertThat(testTaskRecord.allWorkersFinished()).isFalse()
         assertThat(testTaskRecord.lastWorkerCompletionTime()).isEqualTo(Instant.MIN)
 
-        testTaskRecord.get("second").executionStarted()
-        testTaskRecord.get("first").executionStarted()
-        testTaskRecord.get("third").executionStarted()
+        testTaskRecord.get("second")?.executionStarted()
+        testTaskRecord.get("first")?.executionStarted()
+        testTaskRecord.get("third")?.executionStarted()
 
         resetClockTo(200)
-        testTaskRecord.get("second").executionFinished()
+        testTaskRecord.get("second")?.executionFinished()
         assertThat(testTaskRecord.allWorkersFinished()).isFalse()
         assertThat(testTaskRecord.lastWorkerCompletionTime()).isEqualTo(Instant.ofEpochMilli(200))
 
         resetClockTo(220)
-        testTaskRecord.get("first").executionFinished()
+        testTaskRecord.get("first")?.executionFinished()
         assertThat(testTaskRecord.allWorkersFinished()).isFalse()
         assertThat(testTaskRecord.lastWorkerCompletionTime()).isEqualTo(Instant.ofEpochMilli(220))
 
         resetClockTo(215)
-        testTaskRecord.get("third").executionFinished()
+        testTaskRecord.get("third")?.executionFinished()
         assertThat(testTaskRecord.allWorkersFinished()).isTrue()
         assertThat(testTaskRecord.lastWorkerCompletionTime()).isEqualTo(Instant.ofEpochMilli(220))
     }
@@ -107,13 +107,13 @@ class TaskProfilingRecordTest {
         testTaskRecord.addWorker("first")
         testTaskRecord.addWorker("second")
 
-        testTaskRecord.get("second").executionStarted()
-        testTaskRecord.get("first").executionStarted()
+        testTaskRecord.get("second")?.executionStarted()
+        testTaskRecord.get("first")?.executionStarted()
 
         resetClockTo(200)
-        testTaskRecord.get("second").executionFinished()
+        testTaskRecord.get("second")?.executionFinished()
         resetClockTo(220)
-        testTaskRecord.get("first").executionFinished()
+        testTaskRecord.get("first")?.executionFinished()
         assertThat(testTaskRecord.allWorkersFinished()).isTrue()
 
         resetClockTo(135)
@@ -133,13 +133,13 @@ class TaskProfilingRecordTest {
         testTaskRecord.addWorker("second")
 
 
-        testTaskRecord.get("second").executionStarted()
-        testTaskRecord.get("first").executionStarted()
+        testTaskRecord.get("second")?.executionStarted()
+        testTaskRecord.get("first")?.executionStarted()
 
         resetClockTo(200)
-        testTaskRecord.get("second").executionFinished()
+        testTaskRecord.get("second")?.executionFinished()
         resetClockTo(220)
-        testTaskRecord.get("first").executionFinished()
+        testTaskRecord.get("first")?.executionFinished()
         assertThat(testTaskRecord.allWorkersFinished()).isTrue()
 
         resetClockTo(235)
index 93f917d8ed057099ff5156c5d1e3d76b7a21c362..0ab2b0e916bb03bd81eda29c4852b35e33414814 100644 (file)
@@ -40,23 +40,23 @@ class WorkerProfilingRecordTest {
         TaskProfilingRecord.clock = Clock.fixed(Instant.ofEpochMilli(100), ZoneId.systemDefault())
         testTaskRecord.addWorker("first")
         val workerRecord = testTaskRecord.get("first")
-        Truth.assertThat(workerRecord.isStarted()).isFalse()
+        Truth.assertThat(workerRecord?.isStarted()).isFalse()
         TaskProfilingRecord.clock = Clock.fixed(Instant.ofEpochMilli(123), ZoneId.systemDefault())
-        workerRecord.executionStarted()
-        Truth.assertThat(workerRecord.waitTime().toMillis()).isEqualTo(23)
+        workerRecord?.executionStarted()
+        Truth.assertThat(workerRecord?.waitTime()?.toMillis()).isEqualTo(23)
         TaskProfilingRecord.clock = Clock.fixed(Instant.ofEpochMilli(156), ZoneId.systemDefault())
-        workerRecord.executionFinished()
-        Truth.assertThat(workerRecord.duration()).isEqualTo(Duration.ofMillis(33))
+        workerRecord?.executionFinished()
+        Truth.assertThat(workerRecord?.duration()).isEqualTo(Duration.ofMillis(33))
     }
 
     @Test
     fun testUnStartedWorker() {
         testTaskRecord.addWorker("first")
         val workerRecord = testTaskRecord.get("first")
-        Truth.assertThat(workerRecord.isStarted()).isFalse()
-        Truth.assertThat(workerRecord.isFinished()).isFalse()
-        Truth.assertThat(workerRecord.waitTime()).isEqualTo(Duration.ZERO)
-        Truth.assertThat(workerRecord.duration()).isEqualTo(Duration.ZERO)
+        Truth.assertThat(workerRecord?.isStarted()).isFalse()
+        Truth.assertThat(workerRecord?.isFinished()).isFalse()
+        Truth.assertThat(workerRecord?.waitTime()).isEqualTo(Duration.ZERO)
+        Truth.assertThat(workerRecord?.duration()).isEqualTo(Duration.ZERO)
     }
 
     @Test
@@ -64,19 +64,19 @@ class WorkerProfilingRecordTest {
         TaskProfilingRecord.clock = Clock.fixed(Instant.ofEpochMilli(100), ZoneId.systemDefault())
         testTaskRecord.addWorker("first")
         val workerRecord = testTaskRecord.get("first")
-        Truth.assertThat(workerRecord.isStarted()).isFalse()
+        Truth.assertThat(workerRecord?.isStarted()).isFalse()
         TaskProfilingRecord.clock = Clock.fixed(Instant.ofEpochMilli(134), ZoneId.systemDefault())
-        workerRecord.executionStarted()
-        Truth.assertThat(workerRecord.isStarted()).isTrue()
-        Truth.assertThat(workerRecord.isFinished()).isFalse()
-        Truth.assertThat(workerRecord.waitTime()).isEqualTo(Duration.ofMillis(34))
-        Truth.assertThat(workerRecord.duration()).isEqualTo(Duration.ZERO)
+        workerRecord?.executionStarted()
+        Truth.assertThat(workerRecord?.isStarted()).isTrue()
+        Truth.assertThat(workerRecord?.isFinished()).isFalse()
+        Truth.assertThat(workerRecord?.waitTime()).isEqualTo(Duration.ofMillis(34))
+        Truth.assertThat(workerRecord?.duration()).isEqualTo(Duration.ZERO)
     }
 
     @Test(expected = IllegalStateException::class)
     fun testFinishedWithoutStarting() {
         testTaskRecord.addWorker("first")
         val workerRecord = testTaskRecord.get("first")
-        workerRecord.executionFinished()
+        workerRecord?.executionFinished()
     }
 }