Try using GStringChunks in PacketListRecord.

This saves a fair amount of memory in tests here. Loading a large
capture file and sorting on a custom column (tcp.window_size) uses 676
MB before the change and 634 after.

Add notes about possble further improvements:

Roll our own replacement for GStringChunks using wmem_tree.

Have PacketListRecord::columnString return a const char * instead of a
const QByteArray.

Change-Id: Icb36194f5ad290828d7106ccc3bf494d07d76d08
Reviewed-on: https://code.wireshark.org/review/10476
Reviewed-by: Gerald Combs <gerald@wireshark.org>

ui/qt/packet_list_model.cpp

@@ -51,6 +51,7 @@ PacketListModel::PacketListModel(QObject *parent, capture_file *cf) :
     line_spacing_(0)
 {
     setCaptureFile(cf);
+    PacketListRecord::clearStringPool();
     connect(this, SIGNAL(itemHeightChanged(QModelIndex)),
             this, SLOT(emitItemHeightChanged(QModelIndex)),
             Qt::QueuedConnection);
@@ -114,6 +115,7 @@ void PacketListModel::clear() {
     physical_rows_.clear();
     visible_rows_.clear();
     number_to_row_.clear();
+    PacketListRecord::clearStringPool();
     endResetModel();
 }
 

ui/qt/packet_list_record.cpp

@@ -47,6 +47,8 @@ PacketListRecord::PacketListRecord(frame_data *frameData) :
 {
 }
 
+// We might want to return a const char * instead. This would keep us from
+// creating excessive QByteArrays, e.g. in PacketListModel::recordLessThan.
 const QByteArray PacketListRecord::columnString(capture_file *cap_file, int column, bool colorized)
 {
     // packet_list_store.c:packet_list_get_value
@@ -57,7 +59,7 @@ const QByteArray PacketListRecord::columnString(capture_file *cap_file, int colu
     }
 
     bool dissect_color = colorized && !colorized_;
-    if (column >= col_text_.size() || col_text_[column].isNull() || data_ver_ != col_data_ver_ || dissect_color) {
+    if (column >= col_text_.size() || !col_text_[column] || data_ver_ != col_data_ver_ || dissect_color) {
         dissect(cap_file, dissect_color);
     }
 
@@ -172,6 +174,14 @@ void PacketListRecord::dissect(capture_file *cap_file, bool dissect_color)
     ws_buffer_free(&buf);
 }
 
+// This assumes only one packet list. We might want to move this to
+// PacketListModel (or replace this with a wmem allocator).
+struct _GStringChunk *PacketListRecord::string_pool_ = g_string_chunk_new(1 * 1024 * 1024);
+void PacketListRecord::clearStringPool()
+{
+    g_string_chunk_clear(string_pool_);
+}
+
 //#define MINIMIZE_STRING_COPYING 1
 void PacketListRecord::cacheColumnStrings(column_info *cinfo)
 {
@@ -244,23 +254,27 @@ void PacketListRecord::cacheColumnStrings(column_info *cinfo)
             break;
         }
 #else // MINIMIZE_STRING_COPYING
-        // XXX The GTK+ code uses GStringChunk for string storage. It
-        // doesn't appear to be that much faster, but it probably uses
-        // less memory.
-        QByteArray col_text;
+        const char *col_str;
         if (!get_column_resolved(column) && cinfo->col_expr.col_expr_val[column]) {
             /* Use the unresolved value in col_expr_val */
-            col_text = cinfo->col_expr.col_expr_val[column];
+            col_str = cinfo->col_expr.col_expr_val[column];
         } else {
             int text_col = cinfo_column_.value(column, -1);
 
             if (text_col < 0) {
                 col_fill_in_frame_data(fdata_, cinfo, column, FALSE);
             }
-            col_text = cinfo->columns[column].col_data;
+            col_str = cinfo->columns[column].col_data;
+        }
+        // g_string_chunk_insert_const manages a hash table of pointers to
+        // strings:
+        // https://git.gnome.org/browse/glib/tree/glib/gstringchunk.c
+        // We might be better off adding the equivalent functionality to
+        // wmem_tree.
+        col_text_.append(g_string_chunk_insert_const(string_pool_, col_str));
+        for (int i = 0; col_str[i]; i++) {
+            if (col_str[i] == '\n') col_lines++;
         }
-        col_text_.append(col_text);
-        col_lines += col_text.count('\n');
         if (col_lines > lines_) {
             lines_ = col_lines;
             line_count_changed_ = true;

ui/qt/packet_list_record.h

@@ -36,6 +36,7 @@
 #include <QVariant>
 
 struct conversation;
+struct _GStringChunk;
 
 class PacketListRecord
 {
@@ -55,9 +56,11 @@ public:
     inline int lineCount() { return lines_; }
     inline int lineCountChanged() { return line_count_changed_; }
 
+    static void clearStringPool();
+
 private:
     /** The column text for some columns */
-    QList<QByteArray> col_text_;
+    QList<const char *> col_text_;
 
     frame_data *fdata_;
     int lines_;
@@ -75,6 +78,9 @@ private:
 
     void dissect(capture_file *cap_file, bool dissect_color = false);
     void cacheColumnStrings(column_info *cinfo);
+
+    static struct _GStringChunk *string_pool_;
+
 };
 
 #endif // PACKET_LIST_RECORD_H