diff options
author | Gerald Combs <gerald@wireshark.org> | 2016-02-29 16:34:32 -0800 |
---|---|---|
committer | Gerald Combs <gerald@wireshark.org> | 2016-04-06 23:37:34 +0000 |
commit | f611edf4e6061003d4eca88443c54b68d500cecd (patch) | |
tree | 4652e6ef9f9bbc59bdb9a7cfba0dbb5d22d8c53b | |
parent | 2b2a9896811b237346b4f5c35e17a2dfd5f3ea97 (diff) | |
download | wireshark-f611edf4e6061003d4eca88443c54b68d500cecd.tar.gz |
Qt: Follow stream performance improvements.
Make FollowStreamText a subclass of QPlainTextEdit instead of QTextEdit.
For large amounts of text, the former should be less unbearably slow
than the latter. Increase the maximum stream size to 500MB. This isn't
perfect but it's much more usable than 2MB and much easier than the next
step, which is to write our own text display widget.
Process UI events while we fill in the stream data. This gives us
behavior similar to the GTK+ UI and is similar to what we do in other
dialogs.
Switch from g_memdup+g_free to a QByteArray in the Qt UI and GByteArray
in the GTK+ UI.
Don't call readStream twice.
Make sure we free all of our stream data in the Qt UI. This fixes a
serious memory leak.
Ping-Bug: 11777
Change-Id: Ibad9bde86692ae07a80660566d1e661ab8b64601
Reviewed-on: https://code.wireshark.org/review/14271
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Gerald Combs <gerald@wireshark.org>
-rw-r--r-- | ui/gtk/follow_stream.c | 13 | ||||
-rw-r--r-- | ui/qt/follow_stream_dialog.cpp | 104 | ||||
-rw-r--r-- | ui/qt/follow_stream_text.cpp | 18 | ||||
-rw-r--r-- | ui/qt/follow_stream_text.h | 4 | ||||
-rw-r--r-- | ui/qt/wireshark_dialog.h | 8 |
5 files changed, 98 insertions, 49 deletions
diff --git a/ui/gtk/follow_stream.c b/ui/gtk/follow_stream.c index 1413ba54b0..183453fb1d 100644 --- a/ui/gtk/follow_stream.c +++ b/ui/gtk/follow_stream.c @@ -115,7 +115,7 @@ follow_common_read_stream(follow_info_t *follow_info, GList* cur; frs_return_t frs_return; follow_record_t *follow_record; - char *buffer; + GByteArray *buffer = g_byte_array_new(); for (cur = follow_info->payload; cur; cur = g_list_next(cur)) { @@ -134,22 +134,25 @@ follow_common_read_stream(follow_info_t *follow_info, } if (!skip) { - buffer = (char *)g_memdup(follow_record->data->data, + g_byte_array_set_size(buffer, 0); + g_byte_array_append(buffer, follow_record->data->data, follow_record->data->len); frs_return = follow_show(follow_info, follow_print, - buffer, + buffer->data, follow_record->data->len, follow_record->is_server, arg, global_pos, &server_packet_count, &client_packet_count); - g_free(buffer); - if(frs_return == FRS_PRINT_ERROR) + if(frs_return == FRS_PRINT_ERROR) { + g_byte_array_free(buffer, TRUE); return frs_return; + } } } + g_byte_array_free(buffer, TRUE); return FRS_OK; } diff --git a/ui/qt/follow_stream_dialog.cpp b/ui/qt/follow_stream_dialog.cpp index 6aa0162999..15abdd40b2 100644 --- a/ui/qt/follow_stream_dialog.cpp +++ b/ui/qt/follow_stream_dialog.cpp @@ -50,21 +50,24 @@ #include "progress_frame.h" #include "qt_ui_utils.h" +#include <QElapsedTimer> #include <QKeyEvent> #include <QMessageBox> #include <QPrintDialog> #include <QPrinter> +#include <QScrollBar> #include <QTextEdit> #include <QTextStream> // To do: +// - Show text while tapping. // - Instead of calling QMessageBox, display the error message in the text // box and disable the appropriate controls. -// - Draw text by hand similar to ByteViewText. This would let us add -// extra information, e.g. a timestamp column and get rid of the data -// limit. // - Add a progress bar and connect captureCaptureUpdateContinue to it +// Matches SplashOverlay. +static int info_update_freq_ = 100; + FollowStreamDialog::FollowStreamDialog(QWidget &parent, CaptureFile &cf, follow_type_t type) : WiresharkDialog(parent, cf), ui(new Ui::FollowStreamDialog), @@ -72,6 +75,9 @@ FollowStreamDialog::FollowStreamDialog(QWidget &parent, CaptureFile &cf, follow_ follower_(NULL), show_type_(SHOW_ASCII), truncated_(false), + client_packet_count_(0), + server_packet_count_(0), + turns_(0), save_as_(false), use_regex_find_(false) { @@ -217,6 +223,8 @@ void FollowStreamDialog::updateWidgets(bool follow_in_progress) ui->leFind->setEnabled(enable); ui->bFind->setEnabled(enable); b_filter_out_->setEnabled(enable); + b_print_->setEnabled(enable); + b_save_->setEnabled(enable); WiresharkDialog::updateWidgets(); } @@ -358,7 +366,11 @@ void FollowStreamDialog::resetStream() ws_unlink(data_out_filename_.toUtf8().constData()); } for (cur = follow_info_.payload; cur; cur = g_list_next(cur)) { - g_free(cur->data); + follow_record_t *follow_record = (follow_record_t *)cur->data; + if(follow_record->data) { + g_byte_array_free(follow_record->data, TRUE); + } + g_free(follow_record); } g_list_free(follow_info_.payload); follow_info_.payload = NULL; @@ -424,8 +436,13 @@ FollowStreamDialog::readSslStream() guint32 * global_pos; GList * cur; frs_return_t frs_return; + QElapsedTimer elapsed_timer; + + elapsed_timer.start(); for (cur = follow_info_.payload; cur; cur = g_list_next(cur)) { + if (dialogClosed()) break; + SslDecryptedRecord * rec = (SslDecryptedRecord*) cur->data; gboolean include_rec = FALSE; @@ -439,15 +456,22 @@ FollowStreamDialog::readSslStream() (follow_info_.show_stream == FROM_CLIENT); } + QByteArray buffer; if (include_rec) { size_t nchars = rec->data.data_len; - gchar *buffer = (gchar *)g_memdup(rec->data.data, (guint) nchars); + // We want a deep copy. + buffer.clear(); + buffer.append((const char *) rec->data.data, nchars); - frs_return = showBuffer(buffer, nchars, + frs_return = showBuffer(buffer.data(), nchars, rec->is_from_server, rec->packet_num, global_pos); - g_free(buffer); if (frs_return == FRS_PRINT_ERROR) return frs_return; + if (elapsed_timer.elapsed() > info_update_freq_) { + fillHintLabel(ui->teStreamContent->textCursor().position()); + wsApp->processEvents(); + elapsed_timer.start(); + } } } @@ -460,14 +484,11 @@ FollowStreamDialog::followStream() readStream(); } - - -const int FollowStreamDialog::max_document_length_ = 2 * 1000 * 1000; // Just a guess +const int FollowStreamDialog::max_document_length_ = 500 * 1000 * 1000; // Just a guess void FollowStreamDialog::addText(QString text, gboolean is_from_server, guint32 packet_num) { if (save_as_ == true) { - //FILE *fh = (FILE *)arg; size_t nwritten; int FileDescriptor = file_.handle(); FILE* fh = fdopen(dup(FileDescriptor), "wb"); @@ -496,34 +517,32 @@ void FollowStreamDialog::addText(QString text, gboolean is_from_server, guint32 truncated_ = true; } - QColor tagserver_fg = ColorUtils::fromColorT(prefs.st_server_fg); - QColor tagserver_bg = ColorUtils::fromColorT(prefs.st_server_bg); - - QColor tagclient_fg = ColorUtils::fromColorT(prefs.st_client_fg); - QColor tagclient_bg = ColorUtils::fromColorT(prefs.st_client_bg); - + setUpdatesEnabled(false); + int cur_pos = ui->teStreamContent->verticalScrollBar()->value(); ui->teStreamContent->moveCursor(QTextCursor::End); - ui->teStreamContent->setCurrentFont(wsApp->monospaceFont()); - if (is_from_server) - { - ui->teStreamContent->setTextColor(tagserver_fg); - ui->teStreamContent->setTextBackgroundColor(tagserver_bg); - } - else - { - ui->teStreamContent->setTextColor(tagclient_fg); - ui->teStreamContent->setTextBackgroundColor(tagclient_bg); + QTextCharFormat tcf = ui->teStreamContent->currentCharFormat(); + if (is_from_server) { + tcf.setForeground(ColorUtils::fromColorT(prefs.st_server_fg)); + tcf.setBackground(ColorUtils::fromColorT(prefs.st_server_bg)); + } else { + tcf.setForeground(ColorUtils::fromColorT(prefs.st_client_fg)); + tcf.setBackground(ColorUtils::fromColorT(prefs.st_client_bg)); } + ui->teStreamContent->setCurrentCharFormat(tcf); + ui->teStreamContent->insertPlainText(text); - ui->teStreamContent->moveCursor(QTextCursor::End); text_pos_to_packet_[ui->teStreamContent->textCursor().anchor()] = packet_num; if (truncated_) { - ui->teStreamContent->setTextBackgroundColor(ui->teStreamContent->palette().window().color()); - ui->teStreamContent->setTextColor(ui->teStreamContent->palette().windowText().color()); + tcf = ui->teStreamContent->currentCharFormat(); + tcf.setBackground(palette().window().color()); + tcf.setForeground(palette().windowText().color()); ui->teStreamContent->insertPlainText(tr("\n[Stream output truncated]")); ui->teStreamContent->moveCursor(QTextCursor::End); + } else { + ui->teStreamContent->verticalScrollBar()->setValue(cur_pos); } + setUpdatesEnabled(true); } // The following keyboard shortcuts should work (although @@ -610,7 +629,6 @@ FollowStreamDialog::showBuffer(char *buffer, size_t nchars, gboolean is_from_ser * ASCII_TO_EBCDIC(buffer, nchars); */ sanitize_buffer(buffer, nchars); - sanitize_buffer(buffer, nchars); QByteArray ba = QByteArray(buffer, (int)nchars); addText(ba, is_from_server, packet_num); break; @@ -917,10 +935,12 @@ bool FollowStreamDialog::follow(QString previous_filter, bool use_stream_index, setWindowSubtitle(tr("Follow %1 Stream (%2)").arg(proto_get_protocol_short_name(find_protocol_by_id(get_follow_proto_id(follower_)))) .arg(follow_filter)); + ui->cbDirections->blockSignals(true); ui->cbDirections->clear(); ui->cbDirections->addItem(both_directions_string); ui->cbDirections->addItem(client_to_server_string); ui->cbDirections->addItem(server_to_client_string); + ui->cbDirections->blockSignals(false); followStream(); fillHintLabel(-1); @@ -964,9 +984,13 @@ FollowStreamDialog::readFollowStream() GList* cur; frs_return_t frs_return; follow_record_t *follow_record; - char *buffer; + QElapsedTimer elapsed_timer; + + elapsed_timer.start(); for (cur = follow_info_.payload; cur; cur = g_list_next(cur)) { + if (dialogClosed()) break; + follow_record = (follow_record_t *)cur->data; skip = FALSE; if (!follow_record->is_server) { @@ -981,19 +1005,25 @@ FollowStreamDialog::readFollowStream() } } + QByteArray buffer; if (!skip) { - buffer = (char *)g_memdup(follow_record->data->data, - follow_record->data->len); - + // We want a deep copy. + buffer.clear(); + buffer.append((const char *) follow_record->data->data, + follow_record->data->len); frs_return = showBuffer( - buffer, + buffer.data(), follow_record->data->len, follow_record->is_server, follow_record->packet_num, global_pos); - g_free(buffer); if(frs_return == FRS_PRINT_ERROR) return frs_return; + if (elapsed_timer.elapsed() > info_update_freq_) { + fillHintLabel(ui->teStreamContent->textCursor().position()); + wsApp->processEvents(); + elapsed_timer.start(); + } } } diff --git a/ui/qt/follow_stream_text.cpp b/ui/qt/follow_stream_text.cpp index 67c80851fd..e10cf9343f 100644 --- a/ui/qt/follow_stream_text.cpp +++ b/ui/qt/follow_stream_text.cpp @@ -21,31 +21,41 @@ #include "follow_stream_text.h" +#include <wireshark_application.h> + #include <QMouseEvent> #include <QTextCursor> +// To do: +// - Draw text by hand similar to ByteViewText. This would let us add +// extra information, e.g. a timestamp column and get rid of +// max_document_length_ in FollowStreamDialog. + FollowStreamText::FollowStreamText(QWidget *parent) : - QTextEdit(parent) + QPlainTextEdit(parent) { setMouseTracking(true); +// setMaximumBlockCount(1); + QTextDocument *text_doc = document(); + text_doc->setDefaultFont(wsApp->monospaceFont()); } void FollowStreamText::mouseMoveEvent(QMouseEvent *event) { emit mouseMovedToTextCursorPosition(cursorForPosition(event->pos()).position()); - QTextEdit::mouseMoveEvent(event); + QPlainTextEdit::mouseMoveEvent(event); } void FollowStreamText::mousePressEvent(QMouseEvent *event) { emit mouseClickedOnTextCursorPosition(cursorForPosition(event->pos()).position()); - QTextEdit::mousePressEvent(event); + QPlainTextEdit::mousePressEvent(event); } void FollowStreamText::leaveEvent(QEvent *event) { emit mouseMovedToTextCursorPosition(-1); - QTextEdit::leaveEvent(event); + QPlainTextEdit::leaveEvent(event); } /* diff --git a/ui/qt/follow_stream_text.h b/ui/qt/follow_stream_text.h index b6f8a2a598..000ad16c82 100644 --- a/ui/qt/follow_stream_text.h +++ b/ui/qt/follow_stream_text.h @@ -22,9 +22,9 @@ #ifndef FOLLOW_STREAM_TEXT_H #define FOLLOW_STREAM_TEXT_H -#include <QTextEdit> +#include <QPlainTextEdit> -class FollowStreamText : public QTextEdit +class FollowStreamText : public QPlainTextEdit { Q_OBJECT public: diff --git a/ui/qt/wireshark_dialog.h b/ui/qt/wireshark_dialog.h index 4bb816fdc2..9cae588ecf 100644 --- a/ui/qt/wireshark_dialog.h +++ b/ui/qt/wireshark_dialog.h @@ -122,6 +122,12 @@ protected: // XXX Needs a getter? bool file_closed_; + /** + * @brief Check to see if the user has closed (and not minimized) the dialog. + * @return true if the dialog has been closed, false otherwise. + */ + bool dialogClosed() { return dialog_closed_; } + protected slots: /** * @brief Called when the capture file is about to close. This can be @@ -139,7 +145,7 @@ private: QString subtitle_; QList<void *> tap_listeners_; int retap_depth_; - int dialog_closed_; + bool dialog_closed_; private slots: }; |