Comments (5)
I would be glad to make a PR, but I'm a little confused with idea of implementing string-to-int map type which is almost complete copy-paste of string-to-string map. Could you suggest, which solution did you have in mind?
from rcutils.
@almaslov thanks for the issue. I think this is all about rcutils_logging_get_logger_effective_level
is not well optimized, which includes searching the effective FQN for logger, and copying and checking severity string every single time.
rcutils/resource/logging_macros.h.em
Lines 66 to 75 in 8da2aac
I do not have solid idea for this problem. but i would suggest that we should avoid these checks every time. Probably after the initialization, there should be some mechanism to notify the update to the logger, so that we can avoid these all checks.
any thoughts?
from rcutils.
but i would suggest that we should avoid these checks every time
Sounds interesting. The only thing that comes to mind is saving timestamp when global logging state is updated. Logging macro could look something like this:
#define RCUTILS_LOG_COND_NAMED(severity, condition_before, condition_after, name, ...) \
do { \
RCUTILS_LOGGING_AUTOINIT; \
static rcutils_log_location_t __rcutils_logging_location = {__func__, __FILE__, __LINE__}; \
/* New code here */
static timestamp __last_check_timestamp = 0;
static bool __cached_check_result = false;
if (__last_check_timestamp < last_global_logging_update) {
__last_check_timestamp = get_current_time();
__cached_check_result = rcutils_logging_logger_is_enabled_for(name, severity)
}
if (__cached_check_result) { \
condition_before \
rcutils_log(&__rcutils_logging_location, severity, name, __VA_ARGS__); \
condition_after \
} \
} while (0)
So rcutils_logging_logger_is_enabled_for
result can be cached and recalculated only once global state was updated.
The cons is that solution involves modification of macros so it's kinda dangerous since impacts literally all cpp packages. On the other hand the solution I've suggested in issue description doesn't change API, but obviously less effective.
from rcutils.
I took a quick look at this as well. I think there are several problems in here:
rcutils_logging_severity_level_from_string
is doing a string copy and toupper on the string every time it is invoked (Lines 320 to 355 in 8da2aac
- As @fujitatomoya pointed out, we are doing a lot of work in
rcutils_logging_get_logger_effective_level
to try to suss out the parts of the logger that we want. We may be able to optimize this by memoizing that result, though we would still incur overhead in checking on the memoization. We could also reconsider whether we really need that functionality. That is more controversial, since I can definitely see uses for the current system, but logging really should be as simple as possible.
I'm not sure I understand your latest proposal. If we had some kind of global logging update timestamp, wouldn't we just be throttling messages? While that certainly would help, we already have the THROTTLE
versions of the messages.
from rcutils.
@clalancette Thanks for reply.
I agree, that rcutils_logging_severity_level_from_string
has large overhead by copying string and it could be fixed with case-insensitive comparison. But my initial thought was that logging macros shouldn't use (in nested invocations) rcutils_logging_severity_level_from_string
at all.
logging.h
API uses integer log_level
in all functions(except rcutils_logging_severity_level_from_string
). As far as I can see, integer log_level
is converted to string value only for storage purpose. And this is a detail of implementation, not public API. So storing integer value instead of string value could save CPU from unnecessary conversions.
I'm not sure I understand your latest proposal. If we had some kind of global logging update timestamp, wouldn't we just be throttling messages? While that certainly would help, we already have the THROTTLE versions of the messages.
This global logging update timestamp, say last_global_logging_update
, should be updated on logging configuration change, e.g. when rcutils_logging_set_default_logger_level
or rcutils_logging_set_logger_level
called. If logging configuration changes, rcutils_logging_logger_is_enabled_for
can be recalculated only one time, until next configuration update. The idea is last_global_logging_update
affects only rcutils_logging_logger_is_enabled_for
calls, not logging handler calls. So, as you can see in my code snippet I added condition that checks if last_global_logging_update
changed since last macro call.
Actually now I see that there is no way to handle global log level change trough g_rcutils_logging_default_logger_level
global variable which is exposed in logging.h
. So I'm not sure this proposal can be implemented correctly.
from rcutils.
Related Issues (20)
- Calculate the size of the specified directory (include subdirectory) HOT 13
- MSBuild warnings C5105: macro expansion producing 'defined' has undefined behavior HOT 2
- 👩🌾 test_logging_long_messages failing on Dashing HOT 1
- Arch Linux: Compilation error with rcutils in ros2 HOT 5
- cmake warning on OSX: No known linker flag to force library linkage HOT 1
- Warnings while compiling to WebAssembly HOT 2
- error: rcutils/logging_macros.h: No such file or directory HOT 1
- New warning in the packaging linux jobs HOT 3
- logging_macros.h not found HOT 1
- Why does `rcutils_expand_user` fail on non-existent directory? HOT 2
- AddressSanitizer: new-delete-type-mismatch in rcutils_string_map_fini HOT 3
- Human Readable time in logs HOT 12
- "Failed to find exported target names" error while colcon build HOT 3
- Undefined symbol: rcutils_log_internal HOT 2
- Make logging functionality truly thread-safe HOT 3
- performance_test_fixture not found HOT 4
- Valgrind shows leak in rcutils_load_shared_library HOT 1
- Superfluous `va_copy` call in `rcutils_char_array_vsprintf` HOT 2
- Test failure on nightly_linux_address_sanitizer
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from rcutils.