Follow-up refactoring of logging-related parts #1249
Conversation
|
follow up as discussed in #1205 |
|
Pretty big patch. Shall we wait after the release? |
|
We shouldn't. The |
| flags &= (jerry_init_flag_t) ~JERRY_INIT_SHOW_REGEXP_OPCODES; | ||
|
|
||
| JERRY_WARNING_MSG ("Ignoring JERRY_INIT_SHOW_REGEXP_OPCODES flag " | ||
| "because of !REGEXP_DUMP_BYTE_CODE configuration.\n"); |
There was a problem hiding this comment.
Why two lines? Only one line above.
There was a problem hiding this comment.
Written in one line would exceed the 120 chars/line limit.
|
I don't see much risk here. The patch is mostly just renaming. The API change is minor, and happens before the release. LGTM |
akosthekiss
force-pushed
the
logging-followup
branch
from
4d55b59 to
cdf35a2
Compare
|
updated to master and resolved conflicts |
| else if (!strcmp ("--show-regexp-opcodes", argv[i])) | ||
| { | ||
| flags |= JERRY_INIT_SHOW_REGEXP_OPCODES; | ||
| jerry_port_default_set_log_level (JERRY_LOG_LEVEL_DEBUG); |
There was a problem hiding this comment.
Shouldn't it be JERRY_LOG_LEVEL_TRACE ?
There was a problem hiding this comment.
It's true that the regexp engine can log trace level messages but they are mostly "extras" and most of the "important" logs go as debug level. Thus, I wanted to keep the log level implied by "--show-regexp-opcodes" at debug level only as that seamed like a sensible default and aligns with the default implied by other command line switches. Still, should someone want to see the "extras", "--show-regexp-opcodes --log-level 3" is there to use.
akosthekiss
force-pushed
the
logging-followup
branch
from
cdf35a2 to
c338247
Compare
akosthekiss
added
enhancement
akosthekiss
force-pushed
the
logging-followup
branch
from
c338247 to
ee735b0
Compare
|
LGTM |
This patch: * Ensures that all calls to `jerry_port_log` in jerry-core happen via macros defined in jrt.h. Also, it unifies the names of those macros: as `JERRY_ERROR_MSG` and `JERRY_WARNING_MSG` gave a good pattern that was well aligned with the naming scheme of the log level enum, `JERRY_DLOG` and `JERRY_DDLOG` were rewritten to `JERRY_DEBUG_MSG` and `JERRY_TRACE_MSG`. * Ensures that all debug logging code parts of jerry-core (i.e., memory statistics, JS byte-code dumps, and RegExp byte-code dumps) are guarded by macros: `JMEM_STATS`, `PARSER_DUMP_BYTE_CODE`, and `REGEXP_DUMP_BYTE_CODE`, which in turn are controled by cmake build system feature flags `FEATURE_MEM_STATS`, `FEATURE_PARSER_DUMP`, and `FEATURE_REGEXP_DUMP`. * Ensures that all debug logging functionalities can be controled during run time (provided that they were enabled during build time): the engine has `JERRY_INIT_MEM_STATS[_SEPARATE]`, `JERRY_INIT_SHOW_OPCODES`, `JERRY_INIT_SHOW_REGEXP_OPCODES` init flags, and the default unix/linux command line app has corresponding command line switches.` * Drops `FEATURE_LOG`, `JERRY_ENABLE_LOG`, and `JERRY_INIT_ENABLE_LOG`, as their name was misleadingly general, even though they mostly controled the regexp engine only. The above-mentioned `*REGEXP*` things mostly act as their replacements. * Updates build, test, and measurement tool scripts, and documentation. JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu
akosthekiss
force-pushed
the
logging-followup
branch
from
ee735b0 to
a2d5acb
Compare
This patch:
jerry_port_login jerry-core happenvia macros defined in jrt.h. Also, it unifies the names of those
macros: as
JERRY_ERROR_MSGandJERRY_WARNING_MSGgave a goodpattern that was well aligned with the naming scheme of the log
level enum,
JERRY_DLOGandJERRY_DDLOGwere rewritten toJERRY_DEBUG_MSGandJERRY_TRACE_MSG.memory statistics, JS byte-code dumps, and RegExp byte-code
dumps) are guarded by macros:
JMEM_STATS,PARSER_DUMP_BYTE_CODE, andREGEXP_DUMP_BYTE_CODE, which inturn are controled by cmake build system feature flags
FEATURE_MEM_STATS,FEATURE_PARSER_DUMP, andFEATURE_REGEXP_DUMP.during run time (provided that they were enabled during build
time): the engine has
JERRY_INIT_MEM_STATS[_SEPARATE],JERRY_INIT_SHOW_OPCODES,JERRY_INIT_SHOW_REGEXP_OPCODESinitflags, and the default unix/linux command line app has
corresponding command line switches.`
FEATURE_LOG,JERRY_ENABLE_LOG, andJERRY_INIT_ENABLE_LOG, as their name was misleadingly general,even though they mostly controled the regexp engine only. The
above-mentioned
*REGEXP*things mostly act as theirreplacements.
JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu