Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trace-load: improve trace load lookup #688

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

trace-load: improve trace load lookup #688

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
96966fb
tests: fix unit tests on gcc15
alexmohr Oct 20, 2024
013c3aa
trace-load: improve lookup speeds and reduce cpu load caused
alexmohr Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 4 additions & 12 deletions include/dlt/dlt_user.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,9 @@ typedef struct
/* Log Level changed callback */
void (*log_level_changed_callback)(char context_id[DLT_ID_SIZE], uint8_t log_level, uint8_t trace_status);

#ifdef DLT_TRACE_LOAD_CTRL_ENABLE
DltTraceLoadSettings *trace_load_settings; /**< trace load setting for the context */
#endif
} dlt_ll_ts_type;

/**
Expand Down Expand Up @@ -1246,19 +1249,8 @@ DltReturnValue dlt_user_log_resend_buffer(void);
* @param loglevel this is the current log level of the log message to be sent
* @return Value from DltReturnValue enum, DLT_RETURN_TRUE if log level is enabled
*/
static inline DltReturnValue dlt_user_is_logLevel_enabled(DltContext *handle, DltLogLevelType loglevel)
{
if ((loglevel < DLT_LOG_DEFAULT) || (loglevel >= DLT_LOG_MAX))
return DLT_RETURN_WRONG_PARAMETER;
inline DltReturnValue dlt_user_is_logLevel_enabled(DltContext *handle, DltLogLevelType loglevel);

if ((handle == NULL) || (handle->log_level_ptr == NULL))
return DLT_RETURN_WRONG_PARAMETER;

if ((loglevel <= (DltLogLevelType)(*(handle->log_level_ptr))) && (loglevel != DLT_LOG_OFF))
return DLT_RETURN_TRUE;

return DLT_RETURN_LOGGING_DISABLED;
}

# ifdef DLT_TEST_ENABLE
void dlt_user_test_corrupt_user_header(int enable);
Expand Down
67 changes: 48 additions & 19 deletions src/daemon/dlt-daemon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ struct DltTraceLoadLogParams {
};

static DltReturnValue dlt_daemon_output_internal_msg(DltLogLevelType loglevel, const char *text, void *params);
static void dlt_trace_load_free(DltDaemon* daemon);

pthread_rwlock_t trace_load_rw_lock;
#endif
Expand Down Expand Up @@ -1636,6 +1637,9 @@ int main(int argc, char *argv[])
dlt_gateway_deinit(&daemon_local.pGateway, daemon_local.flags.vflag);

dlt_daemon_free(&daemon, daemon_local.flags.vflag);
#ifdef DLT_TRACE_LOAD_CTRL_ENABLE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those lines of code should be warped in 1 function like "dlt_traceload_free"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do, once I'm back in office in December :)

dlt_trace_load_free(&daemon);
#endif

dlt_log(LOG_NOTICE, "Leaving DLT daemon\n");

Expand All @@ -1646,6 +1650,18 @@ int main(int argc, char *argv[])
} /* main() */
#endif

#ifdef DLT_TRACE_LOAD_CTRL_ENABLE
void dlt_trace_load_free(DltDaemon* daemon)
{
if (daemon->preconfigured_trace_load_settings != NULL) {
free(daemon->preconfigured_trace_load_settings);
daemon->preconfigured_trace_load_settings = NULL;
}
pthread_rwlock_destroy(&trace_load_rw_lock);
}
#endif


int dlt_daemon_local_init_p1(DltDaemon *daemon, DltDaemonLocal *daemon_local, int verbose)
{
PRINT_FUNCTION_VERBOSE(verbose);
Expand Down Expand Up @@ -3976,27 +3992,40 @@ bool trace_load_keep_message(DltDaemonApplication *app,
app->apid,
};

DltTraceLoadSettings *trace_load_settings =
dlt_find_runtime_trace_load_settings(
app->trace_load_settings, app->trace_load_settings_count,
app->apid, msg->extendedheader->ctid);

if (trace_load_settings != NULL) {
pthread_rwlock_wrlock(&trace_load_rw_lock);
keep_message = dlt_check_trace_load(
trace_load_settings, mtin, msg->headerextra.tmsp, size,
dlt_daemon_output_internal_msg, (void *)(&params));
pthread_rwlock_unlock(&trace_load_rw_lock);
}
else {
dlt_vlog(
LOG_ERR,
"Failed to lookup trace load limits for %s, "
"dropping message, likely app was not registered properly\n",
app->apid);
keep_message = false;
DltDaemonContext *context = dlt_daemon_context_find(
daemon,
app->apid,
msg->extendedheader->ctid,
daemon->ecuid,
verbose);


if (context == NULL) {
context = dlt_daemon_context_add(
daemon,
app->apid,
msg->extendedheader->ctid,
daemon->default_log_level,
daemon->default_trace_status,
0,
app->user_handle,
"",
daemon->ecuid,
verbose);
if (context == NULL) {
dlt_vlog(LOG_WARNING,
"Can't add ContextID '%.4s' for ApID '%.4s' in %s\n",
msg->extendedheader->ctid, app->apid, __func__);
return false;
}
}

pthread_rwlock_wrlock(&trace_load_rw_lock);
keep_message = dlt_check_trace_load(
context->trace_load_settings, mtin, msg->headerextra.tmsp, size,
dlt_daemon_output_internal_msg, (void *)(&params));
pthread_rwlock_unlock(&trace_load_rw_lock);

return keep_message;
}
#endif
Expand Down
26 changes: 19 additions & 7 deletions src/daemon/dlt_daemon_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,13 +442,6 @@ int dlt_daemon_free(DltDaemon *daemon, int verbose)
free(daemon->app_id_log_level_settings);
}
#endif
#ifdef DLT_TRACE_LOAD_CTRL_ENABLE
if (daemon->preconfigured_trace_load_settings != NULL) {
free(daemon->preconfigured_trace_load_settings);
daemon->preconfigured_trace_load_settings = NULL;
}
pthread_rwlock_destroy(&trace_load_rw_lock);
#endif

if (app_recv_buffer)
free(app_recv_buffer);
Expand Down Expand Up @@ -1134,6 +1127,10 @@ DltDaemonContext *dlt_daemon_context_add(DltDaemon *daemon,
dlt_set_id(context->apid, apid);
dlt_set_id(context->ctid, ctid);

#ifdef DLT_TRACE_LOAD_CTRL_ENABLE
context->trace_load_settings = NULL;
#endif

application->num_contexts++;
new_context = 1;
}
Expand Down Expand Up @@ -1213,6 +1210,21 @@ DltDaemonContext *dlt_daemon_context_add(DltDaemon *daemon,
context->log_level_pos = log_level_pos;
context->user_handle = user_handle;

#ifdef DLT_TRACE_LOAD_CTRL_ENABLE
DltTraceLoadSettings* tl_settings = dlt_find_runtime_trace_load_settings(
application->trace_load_settings,
application->trace_load_settings_count,
application->apid,
context->ctid);
if (tl_settings == NULL) {
dlt_vlog(LOG_WARNING, "failed to find trace load settings for application %s context %s\n",
application->apid, context->ctid);
} else {
context->trace_load_settings = tl_settings;
}
#endif


/* In case a context is loaded from runtime config file,
* the user_handle is 0 and we mark that context as predefined.
*/
Expand Down
5 changes: 4 additions & 1 deletion src/daemon/dlt_daemon_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,10 @@ typedef struct
int user_handle; /**< connection handle for connection to user application */
char *context_description; /**< context description */
int8_t storage_log_level; /**< log level set for offline logstorage */
bool predefined; /**< set to true if this context is predefined by runtime configuration file */
bool predefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Could you explain the purpose of removing the comment of the bool predefined variable? If it was removed intentionally, kindly please consider reverting this line if it is not necessary."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was not used as far I could tell, but maybe missed something?

#ifdef DLT_TRACE_LOAD_CTRL_ENABLE
DltTraceLoadSettings* trace_load_settings; /**< trace load setting for the context */
#endif
} DltDaemonContext;

/*
Expand Down
Loading
Loading