Add feature to remove parser. #1476
Conversation
jerry-core/parser/js/js-parser.c
Outdated
| #include "jcontext.h" | ||
| #include "js-parser-internal.h" | ||
|
|
||
| #ifndef CONFIG_DISABLE_PARSER_BUILTIN |
There was a problem hiding this comment.
Parser is not exactly a built-in. JERRY_DISABLE_PARSER would sound better. Shouldn't we guard the other files too?
There was a problem hiding this comment.
Sounds good, i was not certain how to call it. Let me change it,
I checked on the rest of the code and you guys don't guard the includes, that is why is there after the js-parser-internal.h
sergioamr
force-pushed
the
next/sergio/remove_parser
branch
from
5c41e41 to
db062ac
Compare
|
I mean all files in parser/js should have guards. I know the linker can eliminate dead code, but I would prefer a complete solution. Btw how much is the binary size gain on your system? |
|
It is quite a good saving. Parser On Parser Off I will add the guards to the rest of the files. |
sergioamr
force-pushed
the
next/sergio/remove_parser
branch
from
db062ac to
9112714
Compare
zherczeg
left a comment
zherczeg
left a comment
There was a problem hiding this comment.
I think after a few more changes we are ready to land this patch.
jerry-core/CMakeLists.txt
Outdated
| set(FEATURE_REGEXP_DUMP OFF CACHE BOOL "Enable regexp byte-code dumps?") | ||
| set(FEATURE_SNAPSHOT_SAVE OFF CACHE BOOL "Enable saving snapshot files?") | ||
| set(FEATURE_SNAPSHOT_EXEC OFF CACHE BOOL "Enable executing snapshot files?") | ||
| set(FEATURE_PARSER_DISABLE OFF CACHE BOOL "Disable the parser and 'eval' and use only snapshots?") |
There was a problem hiding this comment.
It seems to me these features are in alphabetic order except the first few (perhaps that should be fixed in a separate patch). You should put this before FEATURE_PARSER_DUMP.
jerry-core/CMakeLists.txt
Outdated
| message(STATUS "FEATURE_REGEXP_DUMP " ${FEATURE_REGEXP_DUMP}) | ||
| message(STATUS "FEATURE_SNAPSHOT_SAVE " ${FEATURE_SNAPSHOT_SAVE}) | ||
| message(STATUS "FEATURE_SNAPSHOT_EXEC " ${FEATURE_SNAPSHOT_EXEC}) | ||
| message(STATUS "FEATURE_PARSER_DISABLE " ${FEATURE_PARSER_DISABLE}) |
jerry-core/CMakeLists.txt
Outdated
| set(DEFINES_JERRY ${DEFINES_JERRY} JERRY_DISABLE_PARSER) | ||
| set(DEFINES_JERRY ${DEFINES_JERRY} JERRY_ENABLE_SNAPSHOT_EXEC) | ||
| endif() | ||
|
|
There was a problem hiding this comment.
It seems these checks also follows the order in the feature list.
There was a problem hiding this comment.
I think it would be better to set the FEATURE_SNAPSHOT_EXEC option to ON in case of disabled parser instead of set this define. I am suggesting to do this check before the status messages or you can create another message which will reports that the FEATURE_SNAPSHOT_EXEC is enabled now (if it was disabled earlier).
jerry-core/parser/js/js-parser.c
Outdated
| JERRY_UNUSED (is_strict); | ||
| JERRY_UNUSED (bytecode_data_p); | ||
|
|
||
| return ecma_raise_syntax_error (""); |
There was a problem hiding this comment.
Please add a valid error message enclosed in ECMA_ERR_MSG, since disabling the parser does not mean disabling error messages.
sergioamr
force-pushed
the
next/sergio/remove_parser
branch
4 times, most recently
from
44dbda7 to
0d919de
Compare
Remove the parser so we can save space if we are only interested in running snapshots. Removing the parser enables the snapshot execution. JerryScript-DCO-1.0-Signed-off-by: Sergio Martinez sergio.martinez.rodriguez@intel.com
sergioamr
force-pushed
the
next/sergio/remove_parser
branch
from
0d919de to
b6940b4
Compare
4 participants
Remove the parser so we can save space if we are only interested in running snapshots.
Removing the parser enables the snapshot execution.
JerryScript-DCO-1.0-Signed-off-by: Sergio Martinez sergio.martinez.rodriguez@intel.com