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

change log level per rt-app invocation #59

Open
v0lker opened this issue Sep 7, 2018 · 5 comments
Open

change log level per rt-app invocation #59

v0lker opened this issue Sep 7, 2018 · 5 comments

Comments

@v0lker
Copy link
Contributor

v0lker commented Sep 7, 2018

currently, it is a compile-time option which i find a bit tedious. could have a command line flag (how about -l ?). i'm assuming that the overhead is acceptable. could provide a PR, if this feature isn't disliked - ?

@jlelli
Copy link
Contributor

jlelli commented Sep 10, 2018

It might be handy. Maybe just follow the -v -vv -vvv common pattern?
Also maybe keep a config option to completely disable logging? Just in case overhead is found not to be so negligible.

@v0lker
Copy link
Contributor Author

v0lker commented Sep 10, 2018

-v is already taken for 'version', so i'd rather leave it in peace. any other suggestion?

as for the overhead and completely disabling:
going from setting the debug level at compile time to doing it dynamically, we'd incur the overhead of an integer comparison for every log_*() invocation. the branch predictor should be very effective here (i think) as the result is always the same.

right now i can think of only one way to have even less overhead and still log dynamically: rewriting the log function in the process' .text in memory on start-up (replacing the function calls with nops). i don't think it is worth the effort, though.

what would you suggest?

@jlelli
Copy link
Contributor

jlelli commented Sep 10, 2018

OK. Please go for -l --log and don't worry about the overhead. We will figure it out later if it turns out to be a real problem, I guess.

@v0lker
Copy link
Contributor Author

v0lker commented Sep 18, 2018

remaining task is to allow a string instead of the numerical value for the -l option. i'll provide a PR soon, unless someone else is eager...

@v0lker
Copy link
Contributor Author

v0lker commented Oct 5, 2018

#68 implements the remaining issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants