epan: Use hash table for dependent frames

Dependent frames list order does not matter and thus significantly
faster data structure can be used. Replace the list with hash table to
avoid excessive CPU usage when opening files containing reassembled
packets consisting of large number of fragments.
This commit is contained in:
Tomasz Moń 2023-01-28 13:41:50 +01:00
parent 5e3d77761b
commit e7d5c49fe1
No known key found for this signature in database
GPG Key ID: 397DFEBE343AD96F
10 changed files with 21 additions and 22 deletions

View File

@ -300,7 +300,7 @@ frame_data_reset(frame_data *fdata)
}
if (fdata->dependent_frames) {
g_slist_free(fdata->dependent_frames);
g_hash_table_destroy(fdata->dependent_frames);
fdata->dependent_frames = NULL;
}
}
@ -314,7 +314,7 @@ frame_data_destroy(frame_data *fdata)
}
if (fdata->dependent_frames) {
g_slist_free(fdata->dependent_frames);
g_hash_table_destroy(fdata->dependent_frames);
fdata->dependent_frames = NULL;
}
}

View File

@ -71,7 +71,7 @@ typedef struct _frame_data {
LLP64 (64-bit Windows) platforms. Put them here, one after the
other, so they don't require padding between them. */
GSList *pfd; /**< Per frame proto data */
GSList *dependent_frames; /**< A list of frames which this one depends on */
GHashTable *dependent_frames; /**< A hash table of frames which this one depends on */
const struct _color_filter *color_filter; /**< Per-packet matching color_filter_t object */
guint16 subnum; /**< subframe number, for protocols that require this */
/* Keep the bitfields below to 16 bits, so this plus the previous field

View File

@ -312,10 +312,10 @@ free_frame_data_sequence(frame_data_sequence *fds)
}
void
find_and_mark_frame_depended_upon(gpointer data, gpointer user_data)
find_and_mark_frame_depended_upon(gpointer key, gpointer value _U_, gpointer user_data)
{
frame_data *dependent_fd;
guint32 dependent_frame = GPOINTER_TO_UINT(data);
guint32 dependent_frame = GPOINTER_TO_UINT(key);
frame_data_sequence *frames = (frame_data_sequence *)user_data;
if (dependent_frame && frames) {
@ -325,7 +325,9 @@ find_and_mark_frame_depended_upon(gpointer data, gpointer user_data)
*/
if (!(dependent_fd->dependent_of_displayed || dependent_fd->passed_dfilter)) {
dependent_fd->dependent_of_displayed = 1;
g_slist_foreach(dependent_fd->dependent_frames, find_and_mark_frame_depended_upon, frames);
if (dependent_fd->dependent_frames) {
g_hash_table_foreach(dependent_fd->dependent_frames, find_and_mark_frame_depended_upon, frames);
}
}
}
}

View File

@ -33,7 +33,7 @@ WS_DLL_PUBLIC frame_data *frame_data_sequence_find(frame_data_sequence *fds,
*/
WS_DLL_PUBLIC void free_frame_data_sequence(frame_data_sequence *fds);
WS_DLL_PUBLIC void find_and_mark_frame_depended_upon(gpointer data, gpointer user_data);
WS_DLL_PUBLIC void find_and_mark_frame_depended_upon(gpointer key, gpointer value, gpointer user_data);
#ifdef __cplusplus

View File

@ -452,13 +452,10 @@ mark_frame_as_depended_upon(frame_data *fd, guint32 frame_num)
/* ws_assert(frame_num < fd->num) - we assume in several other
* places in the code that frames don't depend on future
* frames. */
/* XXX: Looking to see if the frame is already there is slow
* if there's a lot of dependent frames, so this should
* be a hash table or something.
*/
if (g_slist_find(fd->dependent_frames, GUINT_TO_POINTER(frame_num)) == NULL) {
fd->dependent_frames = g_slist_prepend(fd->dependent_frames, GUINT_TO_POINTER(frame_num));
if (fd->dependent_frames == NULL) {
fd->dependent_frames = g_hash_table_new(g_direct_hash, g_direct_equal);
}
g_hash_table_insert(fd->dependent_frames, GUINT_TO_POINTER(frame_num), NULL);
}
}

4
file.c
View File

@ -1194,12 +1194,12 @@ add_packet_to_packet_list(frame_data *fdata, capture_file *cf,
if (dfcode != NULL) {
fdata->passed_dfilter = dfilter_apply_edt(dfcode, edt) ? 1 : 0;
if (fdata->passed_dfilter) {
if (fdata->passed_dfilter && edt->pi.fd->dependent_frames) {
/* This frame passed the display filter but it may depend on other
* (potentially not displayed) frames. Find those frames and mark them
* as depended upon.
*/
g_slist_foreach(edt->pi.fd->dependent_frames, find_and_mark_frame_depended_upon, cf->provider.frames);
g_hash_table_foreach(edt->pi.fd->dependent_frames, find_and_mark_frame_depended_upon, cf->provider.frames);
}
} else
fdata->passed_dfilter = 1;

View File

@ -678,7 +678,7 @@ libwireshark.so.0 libwireshark0 #MINVER#
fetch_tapped_data@Base 1.9.1
filter_expression_iterate_expressions@Base 2.5.0
filter_expression_new@Base 1.9.1
find_and_mark_frame_depended_upon@Base 1.12.0~rc1
find_and_mark_frame_depended_upon@Base 4.1.0
find_capture_dissector@Base 2.3.0
find_conversation@Base 1.9.1
find_conversation_by_id@Base 2.5.0

View File

@ -315,8 +315,8 @@ process_packet(capture_file *cf, epan_dissect_t *edt,
* if a display filter was given and it matches this packet.
*/
if (edt && cf->dfcode) {
if (dfilter_apply_edt(cf->dfcode, edt)) {
g_slist_foreach(edt->pi.fd->dependent_frames, find_and_mark_frame_depended_upon, cf->provider.frames);
if (dfilter_apply_edt(cf->dfcode, edt) && edt->pi.fd->dependent_frames) {
g_hash_table_foreach(edt->pi.fd->dependent_frames, find_and_mark_frame_depended_upon, cf->provider.frames);
}
}

View File

@ -1071,8 +1071,8 @@ process_packet_first_pass(capture_file *cf, epan_dissect_t *edt,
* More importantly, edt.pi.fd.dependent_frames won't be initialized because
* epan hasn't been initialized.
*/
if (edt) {
g_slist_foreach(edt->pi.fd->dependent_frames, find_and_mark_frame_depended_upon, cf->provider.frames);
if (edt && edt->pi.fd->dependent_frames) {
g_hash_table_foreach(edt->pi.fd->dependent_frames, find_and_mark_frame_depended_upon, cf->provider.frames);
}
cf->count++;

View File

@ -3056,8 +3056,8 @@ process_packet_first_pass(capture_file *cf, epan_dissect_t *edt,
* if a display filter was given and it matches this packet.
*/
if (edt && cf->dfcode) {
if (dfilter_apply_edt(cf->dfcode, edt)) {
g_slist_foreach(edt->pi.fd->dependent_frames, find_and_mark_frame_depended_upon, cf->provider.frames);
if (dfilter_apply_edt(cf->dfcode, edt) && edt->pi.fd->dependent_frames) {
g_hash_table_foreach(edt->pi.fd->dependent_frames, find_and_mark_frame_depended_upon, cf->provider.frames);
}
if (selected_frame_number != 0 && selected_frame_number == cf->count + 1) {