libsysprof-analyze: use separate cache for kernel symbols

Additionally this reduces some GHashTable lookup costs by doing it once
for the process-info per-traceable rather than one per instruction pointer
per traceable.
This commit is contained in:
Christian Hergert
2023-05-15 14:30:06 -07:00
parent 909228174e
commit a90b9a2fc7
3 changed files with 55 additions and 50 deletions

View File

@ -31,18 +31,18 @@ G_BEGIN_DECLS
G_DECLARE_FINAL_TYPE (SysprofDocumentSymbols, sysprof_document_symbols, SYSPROF, DOCUMENT_SYMBOLS, GObject) G_DECLARE_FINAL_TYPE (SysprofDocumentSymbols, sysprof_document_symbols, SYSPROF, DOCUMENT_SYMBOLS, GObject)
void _sysprof_document_symbols_new (SysprofDocument *document, void _sysprof_document_symbols_new (SysprofDocument *document,
SysprofStrings *strings, SysprofStrings *strings,
SysprofSymbolizer *symbolizer, SysprofSymbolizer *symbolizer,
GHashTable *pid_to_process_info, GHashTable *pid_to_process_info,
GCancellable *cancellable, GCancellable *cancellable,
GAsyncReadyCallback callback, GAsyncReadyCallback callback,
gpointer user_data); gpointer user_data);
SysprofDocumentSymbols *_sysprof_document_symbols_new_finish (GAsyncResult *result, SysprofDocumentSymbols *_sysprof_document_symbols_new_finish (GAsyncResult *result,
GError **error); GError **error);
SysprofSymbol *_sysprof_document_symbols_lookup (SysprofDocumentSymbols *symbols, SysprofSymbol *_sysprof_document_symbols_lookup (SysprofDocumentSymbols *symbols,
int pid, const SysprofProcessInfo *process_info,
SysprofAddressContext context, SysprofAddressContext context,
SysprofAddress address); SysprofAddress address);
G_END_DECLS G_END_DECLS

View File

@ -32,9 +32,9 @@
struct _SysprofDocumentSymbols struct _SysprofDocumentSymbols
{ {
GObject parent_instance; GObject parent_instance;
SysprofSymbol *context_switches[SYSPROF_ADDRESS_CONTEXT_GUEST_USER+1]; SysprofSymbol *context_switches[SYSPROF_ADDRESS_CONTEXT_GUEST_USER+1];
GHashTable *pid_to_process_info; SysprofSymbolCache *kernel_symbols;
}; };
G_DEFINE_FINAL_TYPE (SysprofDocumentSymbols, sysprof_document_symbols, G_TYPE_OBJECT) G_DEFINE_FINAL_TYPE (SysprofDocumentSymbols, sysprof_document_symbols, G_TYPE_OBJECT)
@ -47,8 +47,6 @@ sysprof_document_symbols_finalize (GObject *object)
for (guint i = 0; i < G_N_ELEMENTS (self->context_switches); i++) for (guint i = 0; i < G_N_ELEMENTS (self->context_switches); i++)
g_clear_object (&self->context_switches[i]); g_clear_object (&self->context_switches[i]);
g_clear_pointer (&self->pid_to_process_info, g_hash_table_unref);
G_OBJECT_CLASS (sysprof_document_symbols_parent_class)->finalize (object); G_OBJECT_CLASS (sysprof_document_symbols_parent_class)->finalize (object);
} }
@ -63,6 +61,7 @@ sysprof_document_symbols_class_init (SysprofDocumentSymbolsClass *klass)
static void static void
sysprof_document_symbols_init (SysprofDocumentSymbols *self) sysprof_document_symbols_init (SysprofDocumentSymbols *self)
{ {
self->kernel_symbols = sysprof_symbol_cache_new ();
} }
typedef struct _Symbolize typedef struct _Symbolize
@ -86,7 +85,8 @@ symbolize_free (Symbolize *state)
} }
static void static void
add_traceable (SysprofStrings *strings, add_traceable (SysprofDocumentSymbols *self,
SysprofStrings *strings,
SysprofProcessInfo *process_info, SysprofProcessInfo *process_info,
SysprofDocumentTraceable *traceable, SysprofDocumentTraceable *traceable,
SysprofSymbolizer *symbolizer) SysprofSymbolizer *symbolizer)
@ -95,7 +95,6 @@ add_traceable (SysprofStrings *strings,
guint64 *addresses; guint64 *addresses;
guint n_addresses; guint n_addresses;
g_assert (process_info != NULL);
g_assert (SYSPROF_IS_DOCUMENT_TRACEABLE (traceable)); g_assert (SYSPROF_IS_DOCUMENT_TRACEABLE (traceable));
g_assert (SYSPROF_IS_SYMBOLIZER (symbolizer)); g_assert (SYSPROF_IS_SYMBOLIZER (symbolizer));
@ -113,16 +112,28 @@ add_traceable (SysprofStrings *strings,
if (sysprof_address_is_context_switch (address, &context)) if (sysprof_address_is_context_switch (address, &context))
{ {
last_context = context; last_context = context;
}
else if (sysprof_symbol_cache_lookup (process_info->symbol_cache, address) != NULL)
{
continue; continue;
} }
if (last_context == SYSPROF_ADDRESS_CONTEXT_KERNEL)
{
g_autoptr(SysprofSymbol) symbol = NULL;
if (sysprof_symbol_cache_lookup (self->kernel_symbols, address) != NULL)
continue;
if ((symbol = _sysprof_symbolizer_symbolize (symbolizer, strings, process_info, last_context, address)))
sysprof_symbol_cache_take (self->kernel_symbols, g_steal_pointer (&symbol));
}
else else
{ {
g_autoptr(SysprofSymbol) symbol = _sysprof_symbolizer_symbolize (symbolizer, strings, process_info, last_context, address); g_autoptr(SysprofSymbol) symbol = NULL;
if (symbol != NULL) if (process_info != NULL &&
sysprof_symbol_cache_lookup (process_info->symbol_cache, address) != NULL)
continue;
if ((symbol = _sysprof_symbolizer_symbolize (symbolizer, strings, process_info, last_context, address)))
sysprof_symbol_cache_take (process_info->symbol_cache, g_steal_pointer (&symbol)); sysprof_symbol_cache_take (process_info->symbol_cache, g_steal_pointer (&symbol));
} }
} }
@ -185,21 +196,15 @@ sysprof_document_symbols_worker (GTask *task,
int pid = sysprof_document_frame_get_pid (SYSPROF_DOCUMENT_FRAME (traceable)); int pid = sysprof_document_frame_get_pid (SYSPROF_DOCUMENT_FRAME (traceable));
SysprofProcessInfo *process_info = g_hash_table_lookup (state->pid_to_process_info, GINT_TO_POINTER (pid)); SysprofProcessInfo *process_info = g_hash_table_lookup (state->pid_to_process_info, GINT_TO_POINTER (pid));
/* We might hit this if we have "Process 0" which may be useful to add_traceable (state->symbols,
* let users know can take processing time. For now, that will just state->strings,
* get skipped unless we deem it really valuable (you'll just jump process_info,
* to "- - Kernel - -" anyway. traceable,
*/ state->symbolizer);
if (process_info == NULL)
continue;
add_traceable (state->strings, process_info, traceable, state->symbolizer);
} }
while (gtk_bitset_iter_next (&iter, &i)); while (gtk_bitset_iter_next (&iter, &i));
} }
state->symbols->pid_to_process_info = g_hash_table_ref (state->pid_to_process_info);
g_task_return_pointer (task, g_task_return_pointer (task,
g_object_ref (state->symbols), g_object_ref (state->symbols),
g_object_unref); g_object_unref);
@ -247,7 +252,7 @@ _sysprof_document_symbols_new_finish (GAsyncResult *result,
/** /**
* _sysprof_document_symbols_lookup: * _sysprof_document_symbols_lookup:
* @self: a #SysprofDocumentSymbols * @self: a #SysprofDocumentSymbols
* @pid: the process identifier * @process_info: (nullable): the process info if necessary
* @context: the #SysprofAddressContext for the address * @context: the #SysprofAddressContext for the address
* @address: a #SysprofAddress to lookup the symbol for * @address: a #SysprofAddress to lookup the symbol for
* *
@ -256,26 +261,24 @@ _sysprof_document_symbols_new_finish (GAsyncResult *result,
* Returns: (transfer none) (nullable): a #SysprofSymbol or %NULL * Returns: (transfer none) (nullable): a #SysprofSymbol or %NULL
*/ */
SysprofSymbol * SysprofSymbol *
_sysprof_document_symbols_lookup (SysprofDocumentSymbols *self, _sysprof_document_symbols_lookup (SysprofDocumentSymbols *self,
int pid, const SysprofProcessInfo *process_info,
SysprofAddressContext context, SysprofAddressContext context,
SysprofAddress address) SysprofAddress address)
{ {
SysprofAddressContext new_context; SysprofAddressContext new_context;
SysprofProcessInfo *process_info;
g_return_val_if_fail (SYSPROF_IS_DOCUMENT_SYMBOLS (self), NULL); g_return_val_if_fail (SYSPROF_IS_DOCUMENT_SYMBOLS (self), NULL);
g_return_val_if_fail (context <= SYSPROF_ADDRESS_CONTEXT_GUEST_USER, NULL); g_return_val_if_fail (context <= SYSPROF_ADDRESS_CONTEXT_GUEST_USER, NULL);
/* TODO: Much better to do decoding in a group of addresses than
* one at a time, so should change this interface a bit.
*/
if (sysprof_address_is_context_switch (address, &new_context)) if (sysprof_address_is_context_switch (address, &new_context))
return self->context_switches[new_context]; return self->context_switches[new_context];
if (!(process_info = g_hash_table_lookup (self->pid_to_process_info, GINT_TO_POINTER (pid)))) if (context == SYSPROF_ADDRESS_CONTEXT_KERNEL)
return NULL; return sysprof_symbol_cache_lookup (self->kernel_symbols, address);
return sysprof_symbol_cache_lookup (process_info->symbol_cache, address); if (process_info != NULL)
return sysprof_symbol_cache_lookup (process_info->symbol_cache, address);
return NULL;
} }

View File

@ -802,6 +802,7 @@ sysprof_document_symbolize_traceable (SysprofDocument *self,
guint n_symbols) guint n_symbols)
{ {
SysprofAddressContext last_context = SYSPROF_ADDRESS_CONTEXT_NONE; SysprofAddressContext last_context = SYSPROF_ADDRESS_CONTEXT_NONE;
const SysprofProcessInfo *process_info;
SysprofAddress *addresses; SysprofAddress *addresses;
guint n_addresses; guint n_addresses;
int pid; int pid;
@ -813,6 +814,7 @@ sysprof_document_symbolize_traceable (SysprofDocument *self,
return 0; return 0;
pid = sysprof_document_frame_get_pid (SYSPROF_DOCUMENT_FRAME (traceable)); pid = sysprof_document_frame_get_pid (SYSPROF_DOCUMENT_FRAME (traceable));
process_info = g_hash_table_lookup (self->pid_to_process_info, GINT_TO_POINTER (pid));
addresses = g_alloca (sizeof (SysprofAddress) * n_symbols); addresses = g_alloca (sizeof (SysprofAddress) * n_symbols);
n_addresses = sysprof_document_traceable_get_stack_addresses (traceable, addresses, n_symbols); n_addresses = sysprof_document_traceable_get_stack_addresses (traceable, addresses, n_symbols);
@ -820,7 +822,7 @@ sysprof_document_symbolize_traceable (SysprofDocument *self,
{ {
SysprofAddressContext context; SysprofAddressContext context;
symbols[i] = _sysprof_document_symbols_lookup (self->symbols, pid, last_context, addresses[i]); symbols[i] = _sysprof_document_symbols_lookup (self->symbols, process_info, last_context, addresses[i]);
if (sysprof_address_is_context_switch (addresses[i], &context)) if (sysprof_address_is_context_switch (addresses[i], &context))
last_context = context; last_context = context;