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.

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

feat: minimize the log count when send messages without destination i… #450

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@
"request": "launch",
"program": "${workspaceFolder}/out/linux/x64/tests/standalone/ten_runtime_smoke_test",
"args": [
"--gtest_filter=PropertyTest.TwoExtensionsSetObject"
"--gtest_filter=SendTest.NotSpecifyDest"
],
"cwd": "${workspaceFolder}/out/linux/x64/tests/standalone/",
"env": {
Expand Down
2 changes: 0 additions & 2 deletions core/include/ten_runtime/binding/cpp/detail/ten_env.h
Original file line number Diff line number Diff line change
Expand Up @@ -835,8 +835,6 @@ class ten_env_t {
// ownership of the cmd to the TEN runtime.
auto *cpp_cmd_ptr = cmd.release();
delete cpp_cmd_ptr;
} else {
TEN_LOGE("Failed to send_cmd: %s", cmd->get_name());
}

return rc;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ class ten_env_tester_t {
// ownership of the cmd to the TEN runtime.
auto *cpp_cmd_ptr = cmd.release();
delete cpp_cmd_ptr;
} else {
TEN_LOGE("Failed to send_cmd: %s", cmd->get_name());
}

return rc;
Expand All @@ -104,8 +102,6 @@ class ten_env_tester_t {
// ownership of the data to the TEN runtime.
auto *cpp_data_ptr = data.release();
delete cpp_data_ptr;
} else {
TEN_LOGE("Failed to send_data: %s", data->get_name());
}

return rc;
Expand All @@ -131,8 +127,6 @@ class ten_env_tester_t {
// back the ownership of the audio_frame to the TEN runtime.
auto *cpp_audio_frame_ptr = audio_frame.release();
delete cpp_audio_frame_ptr;
} else {
TEN_LOGE("Failed to send_audio_frame: %s", audio_frame->get_name());
}

return rc;
Expand All @@ -158,8 +152,6 @@ class ten_env_tester_t {
// back the ownership of the video_frame to the TEN runtime.
auto *cpp_video_frame_ptr = video_frame.release();
delete cpp_video_frame_ptr;
} else {
TEN_LOGE("Failed to send_video_frame: %s", video_frame->get_name());
}

return rc;
Expand Down
12 changes: 5 additions & 7 deletions core/include_internal/ten_runtime/extension/extension.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,13 +160,6 @@ struct ten_extension_t {
ten_hashhandle_t hh_in_extension_store;

ten_extension_context_t *extension_context;

// TODO(Wei): The current situation is if an extension is generated by an
// extension_group addon and the extension is not an addon, then this
// extension does not have extension_info. Two possible solutions:
// 1) For this kind of extension, we can dynamically generate its
// extension_info after its `on_configure_done`.
// 2) Consider removing the extension_group addon mechanism.
ten_extension_info_t *extension_info;

ten_value_t manifest;
Expand Down Expand Up @@ -210,6 +203,11 @@ struct ten_extension_t {
ten_path_timeout_info path_timeout_info;
// @}

// Records the number of occurrences of the error code
// `TEN_ERRNO_MSG_NOT_CONNECTED` for each message name when sending output
// messages.
ten_hashtable_t msg_not_connected_count_map;

void *user_data;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ typedef struct ten_extension_info_t {
ten_sanitizer_thread_check_t thread_check;

ten_string_t extension_addon_name;

ten_loc_t loc;
ten_extension_t *extension;

// The extension_info of the destination extension for each type of message.
ten_all_msg_type_dest_info_t msg_dest_info;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//
// Copyright © 2024 Agora
// This file is part of TEN Framework, an open source project.
// Licensed under the Apache License, Version 2.0, with certain conditions.
// Refer to the "LICENSE" file in the root directory for more information.
//
#pragma once

#include "ten_runtime/ten_config.h"

#include <stddef.h>

#include "ten_utils/container/hash_handle.h"
#include "ten_utils/lib/string.h"

typedef struct ten_extension_t ten_extension_t;

typedef struct ten_extension_output_msg_not_connected_count_t {
ten_hashhandle_t hh_in_map;

ten_string_t msg_name;
size_t count;
} ten_extension_output_msg_not_connected_count_t;

TEN_RUNTIME_PRIVATE_API bool ten_extension_increment_msg_not_connected_count(
ten_extension_t *extension, const char *msg_name);
27 changes: 13 additions & 14 deletions core/src/ten_runtime/extension/extension.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "include_internal/ten_runtime/extension/msg_dest_info/json.h"
#include "include_internal/ten_runtime/extension/msg_dest_info/msg_dest_info.h"
#include "include_internal/ten_runtime/extension/msg_handling.h"
#include "include_internal/ten_runtime/extension/msg_not_connected_cnt.h"
#include "include_internal/ten_runtime/extension/on_xxx.h"
#include "include_internal/ten_runtime/extension_context/extension_context.h"
#include "include_internal/ten_runtime/extension_group/extension_group.h"
Expand Down Expand Up @@ -119,6 +120,10 @@ ten_extension_t *ten_extension_create(

self->ten_env = ten_env_create_for_extension(self);

ten_hashtable_init(
&self->msg_not_connected_count_map,
offsetof(ten_extension_output_msg_not_connected_count_t, hh_in_map));

self->user_data = user_data;

return self;
Expand Down Expand Up @@ -201,6 +206,8 @@ void ten_extension_destroy(ten_extension_t *self) {
self->addon_host = NULL;
}

ten_hashtable_deinit(&self->msg_not_connected_count_map);

TEN_FREE(self);
}

Expand Down Expand Up @@ -498,20 +505,12 @@ static bool ten_extension_determine_out_msg_dest_from_graph(
TEN_MSG_TYPE msg_type = ten_msg_get_type(msg);
const char *msg_name = ten_msg_get_name(msg);

if (err) {
ten_error_set(
err, TEN_ERRNO_MSG_NOT_CONNECTED,
"Failed to find destination of a '%s' message '%s' from graph.",
ten_msg_type_to_string(msg_type), msg_name);
} else {
if (ten_msg_is_cmd_and_result(msg)) {
TEN_LOGD("Failed to find destination of a command '%s' from graph.",
msg_name);
} else {
// The amount of the data-like messages might be huge, so we don't
// dump error logs here to prevent log flooding.
}
}
// In any case, the user needs to be informed about the error where the graph
// does not have a specified destination for the message.
TEN_ASSERT(err, "Should not happen.");
ten_error_set(err, TEN_ERRNO_MSG_NOT_CONNECTED,
"Failed to find destination of a '%s' message '%s' from graph.",
ten_msg_type_to_string(msg_type), msg_name);

return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ ten_extension_info_t *ten_extension_info_create(void) {
ten_string_init(&self->extension_addon_name);

ten_loc_init_empty(&self->loc);
self->extension = NULL;

self->property = ten_value_create_object_with_move(NULL);

Expand Down
80 changes: 80 additions & 0 deletions core/src/ten_runtime/extension/msg_not_connected_cnt.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
//
// Copyright © 2024 Agora
// This file is part of TEN Framework, an open source project.
// Licensed under the Apache License, Version 2.0, with certain conditions.
// Refer to the "LICENSE" file in the root directory for more information.
//
#include "include_internal/ten_runtime/extension/msg_not_connected_cnt.h"

#include "include_internal/ten_runtime/extension/extension.h"
#include "ten_utils/container/hash_table.h"
#include "ten_utils/lib/string.h"
#include "ten_utils/macro/memory.h"

#define TEN_MSG_NOT_CONNECTED_COUNT_RESET_THRESHOLD (1000)

static ten_extension_output_msg_not_connected_count_t *
ten_extension_output_msg_not_connected_count_create(const char *msg_name) {
TEN_ASSERT(msg_name, "Invalid argument.");

ten_extension_output_msg_not_connected_count_t *self =
(ten_extension_output_msg_not_connected_count_t *)TEN_MALLOC(
sizeof(ten_extension_output_msg_not_connected_count_t));
TEN_ASSERT(self, "Failed to allocate memory.");

ten_string_init_from_c_str(&self->msg_name, msg_name, strlen(msg_name));
self->count = 0;

return self;
}

static void ten_extension_output_msg_not_connected_count_destroy(
ten_extension_output_msg_not_connected_count_t *self) {
TEN_ASSERT(self, "Invalid argument.");

ten_string_deinit(&self->msg_name);

TEN_FREE(self);
}

static void ten_extension_output_msg_not_connected_count_hh_destroy(
ten_hashhandle_t *hh) {
TEN_ASSERT(hh, "Should not happen.");

ten_extension_output_msg_not_connected_count_t *entry =
CONTAINER_OF_FROM_FIELD(
hh, ten_extension_output_msg_not_connected_count_t, hh_in_map);
TEN_ASSERT(entry, "Should not happen.");

ten_extension_output_msg_not_connected_count_destroy(entry);
}

bool ten_extension_increment_msg_not_connected_count(ten_extension_t *extension,
const char *msg_name) {
TEN_ASSERT(extension, "Invalid argument.");
TEN_ASSERT(msg_name, "Invalid argument.");

ten_extension_output_msg_not_connected_count_t *entry = NULL;

ten_hashhandle_t *hh = ten_hashtable_find_string(
&extension->msg_not_connected_count_map, msg_name);
if (!hh) {
entry = ten_extension_output_msg_not_connected_count_create(msg_name);
entry->count = 0;

ten_hashtable_add_string(
&extension->msg_not_connected_count_map, &entry->hh_in_map,
ten_string_get_raw_str(&entry->msg_name),
ten_extension_output_msg_not_connected_count_hh_destroy);
} else {
entry = CONTAINER_OF_FROM_FIELD(
hh, ten_extension_output_msg_not_connected_count_t, hh_in_map);
entry->count++;
}

if (entry->count % TEN_MSG_NOT_CONNECTED_COUNT_RESET_THRESHOLD == 0) {
entry->count = 0; // Reset.
return true;
}
return false;
}
20 changes: 10 additions & 10 deletions core/src/ten_runtime/schema_store/store.c
Original file line number Diff line number Diff line change
Expand Up @@ -392,16 +392,16 @@ void ten_schema_store_deinit(ten_schema_store_t *self) {
self->property = NULL;
}

ten_hashtable_clear(&self->cmd_in);
ten_hashtable_clear(&self->cmd_out);
ten_hashtable_clear(&self->data_in);
ten_hashtable_clear(&self->data_out);
ten_hashtable_clear(&self->video_frame_in);
ten_hashtable_clear(&self->video_frame_out);
ten_hashtable_clear(&self->audio_frame_in);
ten_hashtable_clear(&self->audio_frame_out);
ten_hashtable_clear(&self->interface_in);
ten_hashtable_clear(&self->interface_out);
ten_hashtable_deinit(&self->cmd_in);
ten_hashtable_deinit(&self->cmd_out);
ten_hashtable_deinit(&self->data_in);
ten_hashtable_deinit(&self->data_out);
ten_hashtable_deinit(&self->video_frame_in);
ten_hashtable_deinit(&self->video_frame_out);
ten_hashtable_deinit(&self->audio_frame_in);
ten_hashtable_deinit(&self->audio_frame_out);
ten_hashtable_deinit(&self->interface_in);
ten_hashtable_deinit(&self->interface_out);
}

// {
Expand Down
18 changes: 16 additions & 2 deletions core/src/ten_runtime/ten_env/internal/send.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "include_internal/ten_runtime/app/msg_interface/common.h"
#include "include_internal/ten_runtime/common/loc.h"
#include "include_internal/ten_runtime/extension/extension.h"
#include "include_internal/ten_runtime/extension/msg_not_connected_cnt.h"
#include "include_internal/ten_runtime/extension_context/extension_context.h"
#include "include_internal/ten_runtime/extension_thread/extension_thread.h"
#include "include_internal/ten_runtime/msg/cmd_base/cmd_base.h"
Expand All @@ -27,7 +28,7 @@
#include "ten_utils/macro/memory.h"

/**
* @brief All message sending code flows will eventually fall into this
* @brief All message-sending code paths will ultimately converge in this
* function.
*/
static bool ten_send_msg_internal(
Expand All @@ -41,6 +42,9 @@ static bool ten_send_msg_internal(

const bool msg_is_cmd = ten_msg_is_cmd(msg);

// Even if the user does not pass in the `err` parameter, since different
// error scenarios require different handling, we need to create a temporary
// one to obtain the actual error information.
bool err_new_created = false;
if (!err) {
err = ten_error_create();
Expand Down Expand Up @@ -116,7 +120,17 @@ static bool ten_send_msg_internal(
done:
if (!result) {
if (ten_error_errno(err) == TEN_ERRNO_MSG_NOT_CONNECTED) {
TEN_LOGD("Failed to send message: %s", ten_error_errmsg(err));
if (ten_env_get_attach_to(self) == TEN_ENV_ATTACH_TO_EXTENSION) {
ten_extension_t *extension = ten_env_get_attached_extension(self);
TEN_ASSERT(extension, "Should not happen.");

if (ten_extension_increment_msg_not_connected_count(
extension, ten_msg_get_name(msg))) {
TEN_LOGW("Failed to send message: %s", ten_error_errmsg(err));
}
} else {
TEN_LOGE("Failed to send message: %s", ten_error_errmsg(err));
}
} else {
TEN_LOGE("Failed to send message: %s", ten_error_errmsg(err));
}
Expand Down
1 change: 0 additions & 1 deletion core/src/ten_utils/container/hash_bucket.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#include "ten_utils/container/hash_handle.h"
#include "ten_utils/container/hash_table.h"
#include "ten_utils/macro/macros.h"

void ten_hashbucket_add(ten_hashbucket_t *self, ten_hashhandle_t *hh) {
assert(self && hh);
Expand Down
4 changes: 2 additions & 2 deletions core/src/ten_utils/container/hash_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ void ten_hashtable_init(ten_hashtable_t *self, ptrdiff_t hh_offset) {
void ten_hashtable_deinit(ten_hashtable_t *self) {
assert(self);

ten_hashtable_clear(self);

if (self->bkts) {
free(self->bkts);
self->bkts = NULL;
Expand All @@ -205,8 +207,6 @@ void ten_hashtable_clear(ten_hashtable_t *self) {
assert(self);

ten_hashtable_foreach(self, iter) { ten_hashtable_del(self, iter.node); }

ten_hashtable_deinit(self);
}

void ten_hashtable_concat(ten_hashtable_t *self, ten_hashtable_t *target) {
Expand Down
2 changes: 1 addition & 1 deletion core/src/ten_utils/schema/keywords/keyword_properties.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ static void ten_schema_keyword_properties_destroy(ten_schema_keyword_t *self_) {
"Invalid argument.");

ten_signature_set(&self->signature, 0);
ten_hashtable_clear(&self->properties);
ten_hashtable_deinit(&self->properties);
ten_schema_keyword_deinit(&self->hdr);
TEN_FREE(self);
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/ten_utils/schema/schema.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ void ten_schema_deinit(ten_schema_t *self) {
TEN_ASSERT(self && ten_schema_check_integrity(self), "Invalid argument.");

ten_signature_set(&self->signature, 0);
ten_hashtable_clear(&self->keywords);
ten_hashtable_deinit(&self->keywords);
}

static ten_schema_t *ten_schema_create_by_type(const char *type) {
Expand Down
1 change: 1 addition & 0 deletions tests/ten_runtime/smoke/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ ten_executable("ten_runtime_smoke_test") {
"result_conversion",
"return",
"same_thread_ext_on_xxx",
"send",
"standalone_test",
"start_graph",
"suspend_resume",
Expand Down
Loading
Loading