[geary/mjog/921-db-locked: 7/7] Geary.Logging.Record: Stop keeping a ref on logging source objects

[Michael Gratton <mjog src gnome org>]

commit 6d473a2655fe77fdf59476a23f3ff8a5609e608f
Author: Michael Gratton <mike vee net>
Date:   Fri Sep 4 10:09:08 2020 +1000

    Geary.Logging.Record: Stop keeping a ref on logging source objects
    
    When constructing a new record, rather that getting the state object
    from a source and hanging on to it, just keep a few salient details
    and then release it, so we don't keep a ref to the source hanging
    around. This also means we need to pre-fill well-known up front, adding
    some overhead when not logging.
    
    This fixes hitting resource limits like memory and file descriptors
    when lots of objects are logged, as seen when logging is enabled for
    the DB, and in particular for `DatabaseConnection` objects, since each
    one represents an open SQLite database file and related state.
    
    It also ruins the glorious future in which we could inspect logged
    objects in the Inspector in the same way that Firefox allows you to
    inspect objects in the JS console, but c'est la ve. The way forward
    there is probably adding structured data to state objects, allowing
    sources to provide a curated set of properties to log.

 src/engine/util/util-logging.vala | 45 +++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 21 deletions(-)
---
diff --git a/src/engine/util/util-logging.vala b/src/engine/util/util-logging.vala
index c0356862a..97f9ab6a8 100644
--- a/src/engine/util/util-logging.vala
+++ b/src/engine/util/util-logging.vala
@@ -639,6 +639,9 @@ public class Geary.Logging.Record {
     /** The logged message, if any. */
     public string? message = null;
 
+    /** The leaf source object's type, if any. */
+    public GLib.Type? source_type = null;
+
     /** The source filename, if any. */
     public string? source_filename = null;
 
@@ -657,7 +660,7 @@ public class Geary.Logging.Record {
     /** The next log record in the buffer, if any. */
     public Record? next { get; internal set; default = null; }
 
-    private State[] states;
+    private string[] states;
     private bool filled = false;
     private bool old_log_api = false;
 
@@ -674,13 +677,24 @@ public class Geary.Logging.Record {
 
         // Since GLib.LogField only retains a weak ref to its value,
         // find and ref any values we wish to keep around.
-        this.states = new State[fields.length];
+        this.states = new string[fields.length];
         int state_count = 0;
         foreach (GLib.LogField field in fields) {
             switch (field.key) {
             case "GEARY_LOGGING_SOURCE":
-                this.states[state_count++] =
-                    ((Source) field.value).to_logging_state();
+                var state = ((Source) field.value).to_logging_state();
+                GLib.Type type = state.source.get_type();
+                if (state_count == 0) {
+                    this.source_type = type;
+                }
+                this.states[state_count++] = state.format_message();
+                if (type.is_a(typeof(Account))) {
+                    this.account = (Account) state.source;
+                } else if (type.is_a(typeof(ClientService))) {
+                    this.service = (ClientService) state.source;
+                } else if (type.is_a(typeof(Folder))) {
+                    this.folder = (Folder) state.source;
+                }
                 break;
 
             case "GLIB_DOMAIN":
@@ -720,6 +734,7 @@ public class Geary.Logging.Record {
         this.service = other.service;
         this.folder = other.folder;
         this.message = other.message;
+        this.source_type = other.source_type;
         this.source_filename = other.source_filename;
         this.source_line_number = other.source_line_number;
         this.source_function = other.source_function;
@@ -730,6 +745,7 @@ public class Geary.Logging.Record {
         // copying large record chains and code that does copy records
         // can copy only a fixed number.
         // this.next
+        this.next = null;
 
         this.states = other.states;
         this.filled = other.filled;
@@ -745,19 +761,6 @@ public class Geary.Logging.Record {
      * computationally complex and hence is not done by default.
      */
     public void fill_well_known_sources() {
-        if (!this.filled) {
-            foreach (unowned State state in this.states) {
-                GLib.Type type = state.source.get_type();
-                if (type.is_a(typeof(Account))) {
-                    this.account = (Account) state.source;
-                } else if (type.is_a(typeof(ClientService))) {
-                    this.service = (ClientService) state.source;
-                } else if (type.is_a(typeof(Folder))) {
-                    this.folder = (Folder) state.source;
-                }
-            }
-            this.filled = true;
-        }
     }
 
     /** Returns a formatted string representation of this record. */
@@ -786,7 +789,7 @@ public class Geary.Logging.Record {
         // Append in reverse so inner sources appear first
         for (int i = this.states.length - 1; i >= 0; i--) {
             str.append(" [");
-            str.append(this.states[i].format_message());
+            str.append(this.states[i]);
             str.append("]");
         }
 
@@ -804,13 +807,13 @@ public class Geary.Logging.Record {
                 str.append_c(':');
                 str.append(this.source_function.to_string());
             }
-            str.append("]");
-        } else if (this.states.length > 0) {
+            str.append("]: ");
+        } else if (this.source_type != null) {
             // Print the class name of the leaf logging source to at
             // least give a general idea of where the message came
             // from.
             str.append(" ");
-            str.append(this.states[0].source.get_type().name());
+            str.append(this.source_type.name());
             str.append(": ");
         }