From 71b128b06caa8b716e5e9507f2b20a3fc760b8d8 Mon Sep 17 00:00:00 2001
From: Him188 <Him188@mamoe.net>
Date: Sun, 6 Jun 2021 18:26:16 +0800
Subject: [PATCH] Improve ExceptionCollector.collect for better performance,
 close #1308

---
 .../commonMain/kotlin/ExceptionCollector.kt   | 34 ++++++++-----------
 .../mirai/utils/ExceptionCollectorTest.kt     | 13 +++----
 2 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/mirai-core-utils/src/commonMain/kotlin/ExceptionCollector.kt b/mirai-core-utils/src/commonMain/kotlin/ExceptionCollector.kt
index 8b4522c08..929e4a94a 100644
--- a/mirai-core-utils/src/commonMain/kotlin/ExceptionCollector.kt
+++ b/mirai-core-utils/src/commonMain/kotlin/ExceptionCollector.kt
@@ -12,9 +12,7 @@ package net.mamoe.mirai.utils
 import kotlin.contracts.InvocationKind
 import kotlin.contracts.contract
 
-public class ExceptionCollector : Sequence<Throwable> {
-
-    // TODO: 2021/4/20 drop last
+public class ExceptionCollector {
 
     public constructor()
     public constructor(initial: Throwable?) {
@@ -29,20 +27,14 @@ public class ExceptionCollector : Sequence<Throwable> {
 
     @Volatile
     private var last: Throwable? = null
+    private val hashCodes = mutableSetOf<Int>()
 
     @Synchronized
     public fun collect(e: Throwable?) {
         if (e == null) return
-        val last = last
-        if (last != null) {
-            last.itr().forEach { suppressed ->
-                if (suppressed.stackTrace.contentEquals(e.stackTrace)) {
-                    // filter out useless duplicates.
-                    return
-                }
-            }
-            e.addSuppressed(last)
-        }
+        val hashCode = e.stackTrace.contentHashCode()
+        if (!hashCodes.add(hashCode)) return // filter out duplications
+        // we can also check suppressed exceptions of [e] but actual influence would be slight.
         this.last = e
     }
 
@@ -72,14 +64,18 @@ public class ExceptionCollector : Sequence<Throwable> {
     @DslMarker
     private annotation class TerminalOperation
 
-    private fun Throwable.itr(): Iterator<Throwable> {
-        return (sequenceOf(this) + this.suppressed.asSequence().flatMap { it.itr().asSequence() }).iterator()
+    @TestOnly
+    public fun asSequence(): Sequence<Throwable> {
+        fun Throwable.itr(): Iterator<Throwable> {
+            return (sequenceOf(this) + this.suppressed.asSequence().flatMap { it.itr().asSequence() }).iterator()
+        }
+
+        return Sequence {
+            val last = getLast() ?: return@Sequence emptyList<Throwable>().iterator()
+            last.itr()
+        }
     }
 
-    override fun iterator(): Iterator<Throwable> {
-        val last = getLast() ?: return emptyList<Throwable>().iterator()
-        return last.itr()
-    }
 }
 
 /**
diff --git a/mirai-core-utils/src/commonTest/kotlin/net/mamoe/mirai/utils/ExceptionCollectorTest.kt b/mirai-core-utils/src/commonTest/kotlin/net/mamoe/mirai/utils/ExceptionCollectorTest.kt
index 93cf09245..5fcf7eef8 100644
--- a/mirai-core-utils/src/commonTest/kotlin/net/mamoe/mirai/utils/ExceptionCollectorTest.kt
+++ b/mirai-core-utils/src/commonTest/kotlin/net/mamoe/mirai/utils/ExceptionCollectorTest.kt
@@ -14,6 +14,7 @@ import kotlin.test.assertEquals
 import kotlin.test.assertSame
 import kotlin.test.assertTrue
 
+@OptIn(TestOnly::class)
 internal class ExceptionCollectorTest {
 
     @Test
@@ -23,7 +24,7 @@ internal class ExceptionCollectorTest {
         collector.collect(IllegalArgumentException())
 
         assertTrue { collector.getLast() is IllegalArgumentException }
-        assertEquals(1, collector.count())
+        assertEquals(1, collector.asSequence().count())
     }
 
     @Test
@@ -35,7 +36,7 @@ internal class ExceptionCollectorTest {
 
         assertTrue { collector.getLast() is IllegalStateException }
         assertTrue { collector.getLast()!!.suppressed.single() is IllegalArgumentException }
-        assertEquals(2, collector.count())
+        assertEquals(2, collector.asSequence().count())
     }
 
     @Test
@@ -49,7 +50,7 @@ internal class ExceptionCollectorTest {
         assertTrue { collector.getLast() is IllegalStateException }
         assertTrue { collector.getLast()!!.suppressed.single() is IllegalArgumentException }
         assertTrue { collector.getLast()!!.suppressed.single()!!.suppressed.single() is StackOverflowError }
-        assertEquals(3, collector.count())
+        assertEquals(3, collector.asSequence().count())
     }
 
     @Test
@@ -62,9 +63,9 @@ internal class ExceptionCollectorTest {
         collector.collect(exception)
         collector.collect(exception)
 
-        assertSame(exception, collector.last())
+        assertSame(exception, collector.asSequence().last())
         assertEquals(0, collector.getLast()!!.suppressed.size)
-        assertEquals(1, collector.count())
+        assertEquals(1, collector.asSequence().count())
     }
 
     @Test
@@ -82,7 +83,7 @@ internal class ExceptionCollectorTest {
         }
 
         assertEquals(0, collector.getLast()!!.suppressed.size)
-        assertEquals(1, collector.count())
+        assertEquals(1, collector.asSequence().count())
         assertSame(exceptions.first(), collector.getLast())
         assertEquals("#0", collector.getLast()!!.message)
     }