Qt: Cache packet list column strings.

For each displayed packet list row, save a copy of or a pointer to
column strings similar to ui/gtk/packet_list_store.c. This lets us call
epan_dissect_run only once per row.

Bug: 9511
Change-Id: I17e8ebeb5ed70518c9047413c3b2a46f01e904ef
Reviewed-on: https://code.wireshark.org/review/2752
Reviewed-by: Anders Broman <a.broman58@gmail.com>

epan/CMakeLists.txt

@@ -1673,6 +1673,7 @@ set_target_properties(epan PROPERTIES VERSION ${FULL_SO_VERSION} SOVERSION 0)
 
 ABICHECK(libwireshark)
 
+file(GLOB COLUMN_INFO_HEADER column-info.h)
 file(GLOB CRYPT_HEADERS crypt/*.h)
 file(GLOB DFILTER_HEADERS dfilter/*.h ../tools/lemon/cppmagic.h)
 file(GLOB D_HEADERS dissectors/*.h)

ui/qt/packet_list_model.cpp

@@ -21,9 +21,6 @@
 
 #include "packet_list_model.h"
 
-#include <epan/epan_dissect.h>
-#include <epan/column-info.h>
-#include <epan/column.h>
 #include <wsutil/nstime.h>
 #include <epan/prefs.h>
 
@@ -41,12 +38,13 @@
 PacketListModel::PacketListModel(QObject *parent, capture_file *cf) :
     QAbstractItemModel(parent)
 {
-    cap_file_ = cf;
+    setCaptureFile(cf);
 }
 
 void PacketListModel::setCaptureFile(capture_file *cf)
 {
     cap_file_ = cf;
+    resetColumns();
 }
 
 // Packet list records have no children (for now, at least).
@@ -86,9 +84,9 @@ guint PacketListModel::recreateVisibleRows()
     endResetModel();
     beginInsertRows(QModelIndex(), pos, pos);
     foreach (record, physical_rows_) {
-        if (record->getFdata()->flags.passed_dfilter || record->getFdata()->flags.ref_time) {
+        if (record->frameData()->flags.passed_dfilter || record->frameData()->flags.ref_time) {
             visible_rows_ << record;
-            number_to_row_[record->getFdata()->num] = visible_rows_.count() - 1;
+            number_to_row_[record->frameData()->num] = visible_rows_.count() - 1;
         }
     }
     endInsertRows();
@@ -110,6 +108,9 @@ void PacketListModel::clear() {
 void PacketListModel::resetColumns()
 {
     beginResetModel();
+    if (cap_file_) {
+        PacketListRecord::resetColumns(&cap_file_->cinfo);
+    }
     endResetModel();
 }
 
@@ -136,7 +137,7 @@ QVariant PacketListModel::data(const QModelIndex &index, int role) const
     PacketListRecord *record = static_cast<PacketListRecord*>(index.internalPointer());
     if (!record)
         return QVariant();
-    frame_data *fdata = record->getFdata();
+    frame_data *fdata = record->frameData();
     if (!fdata)
         return QVariant();
 
@@ -190,106 +191,15 @@ QVariant PacketListModel::data(const QModelIndex &index, int role) const
         }
         return QColor(color->red >> 8, color->green >> 8, color->blue >> 8);
     case Qt::DisplayRole:
-        // Need packet data -- fall through
-        break;
+    {
+        int column = index.column();
+        //    g_log(NULL, G_LOG_LEVEL_DEBUG, "showing col %d", col_num);
+        return record->columnString(cap_file_, column);
+    }
     default:
         return QVariant();
     }
 
-    int col_num = index.column();
-//    g_log(NULL, G_LOG_LEVEL_DEBUG, "showing col %d", col_num);
-
-    if (col_num > prefs.num_cols)
-        return QVariant();
-
-    epan_dissect_t edt;
-    column_info *cinfo;
-    gboolean create_proto_tree;
-    struct wtap_pkthdr phdr; /* Packet header */
-    Buffer buf;  /* Packet data */
-    gboolean dissect_columns = TRUE; // XXX - Currently only a placeholder
-
-    if (dissect_columns && cap_file_)
-        cinfo = &cap_file_->cinfo;
-    else
-        cinfo = NULL;
-
-    memset(&phdr, 0, sizeof(struct wtap_pkthdr));
-
-    buffer_init(&buf, 1500);
-    if (!cap_file_ || !cf_read_record_r(cap_file_, fdata, &phdr, &buf)) {
-        /*
-         * Error reading the record.
-         *
-         * Don't set the color filter for now (we might want
-         * to colorize it in some fashion to warn that the
-         * row couldn't be filled in or colorized), and
-         * set the columns to placeholder values, except
-         * for the Info column, where we'll put in an
-         * error message.
-         */
-        if (dissect_columns) {
-            col_fill_in_error(cinfo, fdata, FALSE, FALSE /* fill_fd_columns */);
-
-            //            for(gint col = 0; col < cinfo->num_cols; ++col) {
-            //                /* Skip columns based on frame_data because we already store those. */
-            //                if (!col_based_on_frame_data(cinfo, col))
-            //                    packet_list_change_record(packet_list, record->physical_pos, col, cinfo);
-            //            }
-            //            record->columnized = TRUE;
-        }
-        if (enable_color_) {
-            fdata->color_filter = NULL;
-            //            record->colorized = TRUE;
-        }
-        buffer_free(&buf);
-        return QVariant();	/* error reading the record */
-    }
-
-    create_proto_tree = (color_filters_used() && enable_color_) ||
-                        (have_custom_cols(cinfo) && dissect_columns);
-
-    epan_dissect_init(&edt, cap_file_->epan,
-                      create_proto_tree,
-                      FALSE /* proto_tree_visible */);
-
-    if (enable_color_)
-        color_filters_prime_edt(&edt);
-    if (dissect_columns)
-        col_custom_prime_edt(&edt, cinfo);
-
-    epan_dissect_run(&edt, cap_file_->cd_t, &phdr, frame_tvbuff_new_buffer(fdata, &buf), fdata, cinfo);
-
-    if (enable_color_)
-        fdata->color_filter = color_filters_colorize_packet(&edt);
-
-    if (dissect_columns) {
-        /* "Stringify" non frame_data vals */
-        epan_dissect_fill_in_columns(&edt, FALSE, FALSE /* fill_fd_columns */);
-
-        //            for(col = 0; col < cinfo->num_cols; ++col) {
-        //                    /* Skip columns based on frame_data because we already store those. */
-        //                    if (!col_based_on_frame_data(cinfo, col))
-        //                            packet_list_change_record(packet_list, record->physical_pos, col, cinfo);
-        //            }
-//        g_log(NULL, G_LOG_LEVEL_DEBUG, "d_c %d: %s", col_num, cinfo->col_data[col_num]);
-    }
-
-    //    if (dissect_columns)
-    //            record->columnized = TRUE;
-    //    if (enable_color_)
-    //            record->colorized = TRUE;
-
-    epan_dissect_cleanup(&edt);
-    buffer_free(&buf);
-
-    switch (role) {
-    case Qt::DisplayRole:
-        return record->data(col_num, cinfo);
-        break;
-    default:
-        break;
-    }
     return QVariant();
 }
 
@@ -334,14 +244,14 @@ frame_data *PacketListModel::getRowFdata(int row) {
     PacketListRecord *record = visible_rows_[row];
     if (!record)
         return NULL;
-    return record->getFdata();
+    return record->frameData();
 }
 
 int PacketListModel::visibleIndexOf(frame_data *fdata) const
 {
     int row = 0;
     foreach (PacketListRecord *record, visible_rows_) {
-        if (record->getFdata() == fdata) {
+        if (record->frameData() == fdata) {
             return row;
         }
         row++;

ui/qt/packet_list_record.cpp

@@ -21,30 +21,228 @@
 
 #include "packet_list_record.h"
 
+#include <file.h>
+
+#include <epan/epan_dissect.h>
+#include <epan/column-info.h>
+#include <epan/column.h>
+
+#include "color.h"
+#include "color_filters.h"
+#include "frame_tvbuff.h"
+
+#include <QStringList>
+
+QMap<int, int> cinfo_column_;
+
 PacketListRecord::PacketListRecord(frame_data *frameData) :
-    //col_text_(NULL),
-    //col_text_len_(NULL),
-    fdata_(frameData)
+    fdata_(frameData),
+//    columnized_(false),
+    colorized_(false)
 {
 }
 
-QVariant PacketListRecord::data(int col_num, column_info *cinfo) const
+QVariant PacketListRecord::columnString(capture_file *cap_file, int column)
 {
+    // packet_list_store.c:packet_list_get_value
     g_assert(fdata_);
 
-    if (!cinfo)
+    if (!cap_file || column < 0 || column > cap_file->cinfo.num_cols) {
         return QVariant();
+    }
 
-    if (col_based_on_frame_data(cinfo, col_num))
-        col_fill_in_frame_data(fdata_, cinfo, col_num, FALSE);
+    if (column >= col_text_.size() || col_text_[column].isNull() || !colorized_) {
+        dissect(cap_file, !colorized_);
+    }
 
-    return cinfo->col_data[col_num];
+    return col_text_.value(column, QByteArray());
 }
 
-frame_data *PacketListRecord::getFdata() {
+frame_data *PacketListRecord::frameData() {
     return fdata_;
 }
 
+void PacketListRecord::resetColumns(column_info *cinfo)
+{
+    if (!cinfo) {
+        return;
+    }
+
+    cinfo_column_.clear();
+    int i, j;
+    for (i = 0, j = 0; i < cinfo->num_cols; i++) {
+        if (!col_based_on_frame_data(cinfo, i)) {
+            cinfo_column_[i] = j;
+            j++;
+        }
+    }
+}
+
+void PacketListRecord::dissect(capture_file *cap_file, bool dissect_color)
+{
+    // packet_list_store.c:packet_list_dissect_and_cache_record
+    epan_dissect_t edt;
+    column_info *cinfo = NULL;
+    gboolean create_proto_tree;
+    struct wtap_pkthdr phdr; /* Packet header */
+    Buffer buf; /* Packet data */
+    gboolean dissect_columns = col_text_.isEmpty();
+
+    if (!cap_file) {
+        return;
+    }
+
+    memset(&phdr, 0, sizeof(struct wtap_pkthdr));
+
+    if (dissect_columns) {
+        cinfo = &cap_file->cinfo;
+    }
+
+    buffer_init(&buf, 1500);
+    if (!cf_read_record_r(cap_file, fdata_, &phdr, &buf)) {
+        /*
+         * Error reading the record.
+         *
+         * Don't set the color filter for now (we might want
+         * to colorize it in some fashion to warn that the
+         * row couldn't be filled in or colorized), and
+         * set the columns to placeholder values, except
+         * for the Info column, where we'll put in an
+         * error message.
+         */
+        if (dissect_columns) {
+            col_fill_in_error(cinfo, fdata_, FALSE, FALSE /* fill_fd_columns */);
+
+            cacheColumnStrings(cinfo);
+        }
+        if (dissect_color) {
+            fdata_->color_filter = NULL;
+            colorized_ = TRUE;
+        }
+        buffer_free(&buf);
+        return;    /* error reading the record */
+    }
+
+    create_proto_tree = (dissect_color && color_filters_used()) ||
+                        (dissect_columns && have_custom_cols(cinfo));
+
+    epan_dissect_init(&edt, cap_file->epan,
+                      create_proto_tree,
+                      FALSE /* proto_tree_visible */);
+
+    if (dissect_color)
+        color_filters_prime_edt(&edt);
+    if (dissect_columns)
+        col_custom_prime_edt(&edt, cinfo);
+
+    /*
+     * XXX - need to catch an OutOfMemoryError exception and
+     * attempt to recover from it.
+     */
+    epan_dissect_run(&edt, cap_file->cd_t, &phdr, frame_tvbuff_new_buffer(fdata_, &buf), fdata_, cinfo);
+
+    if (dissect_color)
+        fdata_->color_filter = color_filters_colorize_packet(&edt);
+
+    if (dissect_columns) {
+        /* "Stringify" non frame_data vals */
+        epan_dissect_fill_in_columns(&edt, FALSE, FALSE /* fill_fd_columns */);
+        cacheColumnStrings(cinfo);
+    }
+
+    if (dissect_color) {
+        colorized_ = true;
+    }
+
+    epan_dissect_cleanup(&edt);
+    buffer_free(&buf);
+}
+
+//#define MINIMIZE_STRING_COPYING 1
+void PacketListRecord::cacheColumnStrings(column_info *cinfo)
+{
+    // packet_list_store.c:packet_list_change_record(PacketList *packet_list, PacketListRecord *record, gint col, column_info *cinfo)
+    if (!cinfo) {
+        return;
+    }
+
+    col_text_.clear();
+
+    for (int column = 0; column < cinfo->num_cols; ++column) {
+
+#ifdef MINIMIZE_STRING_COPYING
+        int text_col = cinfo_column_.value(column, -1);
+
+        /* Column based on frame_data or it already contains a value */
+        if (text_col < 0) {
+            col_fill_in_frame_data(fdata_, cinfo, column, FALSE);
+            col_text_.append(cinfo->col_data[column]);
+            continue;
+        }
+
+        switch (cinfo->col_fmt[column]) {
+        case COL_DEF_SRC:
+        case COL_RES_SRC:        /* COL_DEF_SRC is currently just like COL_RES_SRC */
+        case COL_UNRES_SRC:
+        case COL_DEF_DL_SRC:
+        case COL_RES_DL_SRC:
+        case COL_UNRES_DL_SRC:
+        case COL_DEF_NET_SRC:
+        case COL_RES_NET_SRC:
+        case COL_UNRES_NET_SRC:
+        case COL_DEF_DST:
+        case COL_RES_DST:        /* COL_DEF_DST is currently just like COL_RES_DST */
+        case COL_UNRES_DST:
+        case COL_DEF_DL_DST:
+        case COL_RES_DL_DST:
+        case COL_UNRES_DL_DST:
+        case COL_DEF_NET_DST:
+        case COL_RES_NET_DST:
+        case COL_UNRES_NET_DST:
+        case COL_PROTOCOL:
+        case COL_INFO:
+        case COL_IF_DIR:
+        case COL_DCE_CALL:
+        case COL_8021Q_VLAN_ID:
+        case COL_EXPERT:
+        case COL_FREQ_CHAN:
+            if (cinfo->col_data[column] && cinfo->col_data[column] != cinfo->col_buf[column]) {
+                /* This is a constant string, so we don't have to copy it */
+                // XXX - ui/gtk/packet_list_store.c uses G_MAXUSHORT. We don't do proper UTF8
+                // truncation in either case.
+                int col_text_len = MIN(qstrlen(cinfo->col_data[column]) + 1, COL_MAX_INFO_LEN);
+                col_text_.append(QByteArray::fromRawData(cinfo->col_data[column], col_text_len));
+                break;
+            }
+            /* !! FALL-THROUGH!! */
+
+        default:
+            if (!get_column_resolved(column) && cinfo->col_expr.col_expr_val[column]) {
+                /* Use the unresolved value in col_expr_val */
+                // XXX Use QContiguousCache?
+                col_text_.append(cinfo->col_expr.col_expr_val[column]);
+            } else {
+                col_text_.append(cinfo->col_data[column]);
+            }
+            break;
+        }
+#else // MINIMIZE_STRING_COPYING
+        // XXX Use QContiguousCache?
+        if (!get_column_resolved(column) && cinfo->col_expr.col_expr_val[column]) {
+            /* Use the unresolved value in col_expr_val */
+            col_text_.append(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_.append(cinfo->col_data[column]);
+        }
+#endif // MINIMIZE_STRING_COPYING
+    }
+}
+
 /*
  * Editor modelines
  *

ui/qt/packet_list_record.h

@@ -26,9 +26,12 @@
 
 #include <glib.h>
 
+#include "cfile.h"
+
 #include <epan/column-info.h>
 #include <epan/packet.h>
 
+#include <QByteArray>
 #include <QList>
 #include <QVariant>
 
@@ -36,21 +39,22 @@ class PacketListRecord
 {
 public:
     PacketListRecord(frame_data *frameData);
-    QVariant data(int col_num, column_info *cinfo) const;
-    frame_data *getFdata();
+    // Return the string value for a column. Data is cached if possible.
+    QVariant columnString(capture_file *cap_file, int column);
+    frame_data *frameData();
+
+    static void resetColumns(column_info *cinfo);
 
 private:
     /** The column text for some columns */
-    //gchar **col_text_;
-    /**< The length of the column text strings in 'col_text' */
-    //guint *col_text_len_;
+    QList<QByteArray> col_text_;
 
     frame_data *fdata_;
 
     /** Has this record been columnized? */
-    //gboolean columnized_;
+//    gboolean columnized_;
     /** Has this record been colorized? */
-    //gboolean colorized_;
+    bool colorized_;
 
     /* admin stuff used by the custom list model */
     /** position within the physical array */
@@ -58,6 +62,9 @@ private:
     /** position within the visible array */
     //gint visible_pos_;
 
+    void dissect(capture_file *cap_file, bool dissect_color = false);
+    void cacheColumnStrings(column_info *cinfo);
+
 };
 
 #endif // PACKET_LIST_RECORD_H

ui/qt/related_packet_delegate.cpp

@@ -40,7 +40,7 @@ void RelatedPacketDelegate::paint(QPainter *painter, const QStyleOptionViewItem
 
     frame_data *fd;
     PacketListRecord *record = static_cast<PacketListRecord*>(index.internalPointer());
-    if (!record || (fd = record->getFdata()) == NULL) {
+    if (!record || (fd = record->frameData()) == NULL) {
         return;
     }