From 5f1472dbb75717942cd72c86f9c5af3406ea9c7d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit-Claudel?= <clement.pit-claudel@epfl.ch>
Date: Sun, 22 Dec 2024 00:19:54 +0100
Subject: [PATCH] server/gc: User time.Instant instead of milliseconds and add
 logging

---
 .../webapp/server/web/GarbageCollector.scala  | 37 +++++++++++++------
 .../cs214/webapp/server/web/ServerApp.scala   | 11 +++---
 .../cs214/webapp/server/web/WebServer.scala   |  4 +-
 3 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/jvm/src/main/scala/cs214/webapp/server/web/GarbageCollector.scala b/jvm/src/main/scala/cs214/webapp/server/web/GarbageCollector.scala
index b8e921f..e97ea26 100644
--- a/jvm/src/main/scala/cs214/webapp/server/web/GarbageCollector.scala
+++ b/jvm/src/main/scala/cs214/webapp/server/web/GarbageCollector.scala
@@ -3,26 +3,41 @@ package server.web
 
 import scala.collection.concurrent
 import scala.concurrent.duration.*
+import scala.jdk.DurationConverters.*
+
+import java.time.Instant
 
 object GarbageCollector:
-  private val GC_TICK = 1.minutes
-  private val GC_CREATION_TIME_GRACE_PERIOD = 1.hour
-  private val GC_LAST_ACTIVITY_GRACE_PERIOD = 3.days
+  private val GC_TICK = 5.minutes
+  private val GC_CREATION_TIME_GRACE_PERIOD = 1.hour.toJava
+  private val GC_LAST_ACTIVITY_GRACE_PERIOD = 3.days.toJava
 
-  def garbageCollect(): Unit = // Not safe, needs lock or care; too slow?
-    val now = System.currentTimeMillis()
-    val lastActivityThreshold = now - GC_LAST_ACTIVITY_GRACE_PERIOD.toMillis
-    val creationTimeThreshold = now - GC_CREATION_TIME_GRACE_PERIOD.toMillis
+  def garbageCollect(): Unit =
+    val now = Instant.now()
+    val lastActivityThreshold = now.minus(GC_LAST_ACTIVITY_GRACE_PERIOD)
+    val creationTimeThreshold = now.minus(GC_CREATION_TIME_GRACE_PERIOD)
+    println(f"[gc] Collection started at $now")
+    println(f"[gc] Collection criteria:\n" +
+            f"[gc]   Created before: $creationTimeThreshold\n" +
+            f"[gc]   Last active before: $lastActivityThreshold")
     WebServer.instances.snapshot()
       .values.filter: inst =>
-        inst.lastActivity < collectInactiveBefore &&
-        inst.creationTime < collectNeverConnectedBefore
+        val lastActivity = inst.lastActivity
+        lastActivity.isBefore(lastActivityThreshold) &&
+        inst.creationTime.isBefore(creationTimeThreshold) && {
+          println(f"[gc] Marking ${inst.appInfo.id}/${inst.instanceId} for collection:\n" +
+                  f"[gc]   Activity: $lastActivity < $lastActivityThreshold\n" +
+                  f"[gc]   Creation: ${inst.creationTime} < $creationTimeThreshold")
+          true
+        }
       .toList.foreach: inst =>
-        println(f"[gc] Stopping ${inst.appInfo.name}/${inst.instanceId}.")
+        println(f"[gc] Stopping ${inst.appInfo.id}/${inst.instanceId}.")
         WebServer.shutdownApp(inst.instanceId)
+    println("[gc] Collection complete")
 
   private val clock =
     val scheduler = Clock.newScheduler()
     Clock("gc", GC_TICK.toMillis)(using scheduler)(garbageCollect())
 
-  clock.start()
+  def start() = clock.start()
+  def shutdown() = clock.shutdown()
diff --git a/jvm/src/main/scala/cs214/webapp/server/web/ServerApp.scala b/jvm/src/main/scala/cs214/webapp/server/web/ServerApp.scala
index a0ed290..4b2313b 100644
--- a/jvm/src/main/scala/cs214/webapp/server/web/ServerApp.scala
+++ b/jvm/src/main/scala/cs214/webapp/server/web/ServerApp.scala
@@ -3,7 +3,8 @@ package server
 package web
 
 import java.util.concurrent.ScheduledExecutorService
-import java.util.concurrent.atomic.AtomicLong
+import java.util.concurrent.atomic.AtomicReference
+import java.time.Instant
 
 import scala.collection.mutable
 import scala.util.{Success, Failure, Try}
@@ -58,11 +59,11 @@ private[web] abstract class ServerApp:
     */
   def transition(clients: Seq[UserId])(userId: UserId, eventJs: ujson.Value): Try[PendingActions]
 
-  val creationTime = System.currentTimeMillis()
+  val creationTime = Instant.now
 
-  private var _lastActivity = AtomicLong(0)
-  def recordActivity() = lastActivity.set(System.currentTimeMillis())
-  def lastActivity = lastActivity.get()
+  private var _lastActivity = AtomicReference(Instant.MIN)
+  def recordActivity() = _lastActivity.set(Instant.now)
+  def lastActivity = _lastActivity.get()
 
   private val channels =
     mutable.Map[UserId, mutable.Set[cask.WsChannelActor]]().withDefault(_ => mutable.Set())
diff --git a/jvm/src/main/scala/cs214/webapp/server/web/WebServer.scala b/jvm/src/main/scala/cs214/webapp/server/web/WebServer.scala
index c33e612..88e1e61 100644
--- a/jvm/src/main/scala/cs214/webapp/server/web/WebServer.scala
+++ b/jvm/src/main/scala/cs214/webapp/server/web/WebServer.scala
@@ -23,8 +23,6 @@ private case class Wordlist(fname: String):
 
 /** Contains the web server state and functionalities */
 object WebServer:
-  GarbageCollector.start()
-
   private val instanceIdsFileName = "eff_short_wordlist_1.txt" // Where to find the list of possible instance ids
   private val instanceIdLength = 4 // How many words do we concatenate for the instance id
   private val instanceIdWords = Wordlist(instanceIdsFileName)
@@ -69,3 +67,5 @@ object WebServer:
     instances.remove(instanceId).map: app =>
       app.shutdown()
       println(s"[${app.appInfo.id}][$instanceId] shut down")
+
+  GarbageCollector.start()
-- 
GitLab