Make logging an optional feature and disable it by default #2449
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Results on binary size (and on RSS in consequence):
Binary sizes (bytes) (~1.8k gain) |
zherczeg
requested changes
Aug 7, 2018
jerry-core/CMakeLists.txt
Outdated
| @@ -66,6 +67,12 @@ if(JERRY_CMDLINE_SNAPSHOT) | |||
| set(FEATURE_SNAPSHOT_SAVE_MESSAGE " (FORCED BY SNAPSHOT TOOL)") | |||
| endif() | |||
|
|
|||
| if(FEATURE_DEBUGGER OR FEATURE_MEM_STATS OR FEATURE_PARSER_DUMP OR FEATURE_REGEXP_DUMP) | |||
| set(FEATURE_LOGGING ON) | |||
There was a problem hiding this comment.
Debugging should not force logging.
jerry-core/parser/js/common.c
Outdated
| @@ -74,6 +76,7 @@ util_print_number (ecma_number_t num_p) /**< number to print */ | |||
| lit_utf8_byte_t str_buf[ECMA_MAX_CHARS_IN_STRINGIFIED_NUMBER]; | |||
| lit_utf8_size_t str_size = ecma_number_to_utf8_string (num_p, str_buf, sizeof (str_buf)); | |||
| str_buf[str_size] = 0; | |||
| JERRY_UNUSED (str_buf); // potentially | |||
There was a problem hiding this comment.
We will forget adding these macros. Just like asserts, logging should reference its arguments.
jerry-core/parser/js/js-parser.c
Outdated
| JERRY_DEBUG_MSG (is_constructor ? "\n--- Class constructor parsing end ---\n\n" | ||
| : "\n--- Function parsing end ---\n\n"); | ||
| JERRY_DEBUG_MSG ("\n--- %s parsing end ---\n\n", | ||
| context_p->status_flags & PARSER_CLASS_CONSTRUCTOR ? "Class constructor" |
There was a problem hiding this comment.
Brackets around context_p->status_flags & PARSER_CLASS_CONSTRUCTOR.
|
PR updated according to the reviews Note 1: "Just like asserts, logging should reference its arguments." This needed some macro black magic as the classic Note 2: As of late, the ESP target has been erroring. It makes all PRs red (including this one). Hope it's only some instability that goes away soon. |
JerryScript-DCO-1.0-Signed-off-by: Akos Kiss [email protected]
akosthekiss
force-pushed
the
disable-log
branch
from
654c19e to
817fb58
Compare
|
Commits squashed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JerryScript-DCO-1.0-Signed-off-by: Akos Kiss [email protected]