Skip to content

Few improvements for RegExp #947

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

Merged

Conversation

LaszloLango
Copy link
Contributor

Added eviction mechanism to RegExp cache and small
refactoring. Fixed a bug when logging is enabled.

Related issue: #927

JerryScript-DCO-1.0-Signed-off-by: László Langó llango.u-szeged@partner.samsung.com

@LaszloLango LaszloLango added enhancement An improvement ecma builtins Related to ECMA built-in routines labels Mar 9, 2016
if (cache_idx < RE_CACHE_SIZE)
if (cache_idx >= RE_CACHE_SIZE)
{
if (re_cache_idx < 1u)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, in these two ifs, we are checking whether cache_idx == RE_CACHE_SIZE and re_cache_idx == 0, aren't we? (I mean, there is no way for cache_idx to be striclty larger that RE_CACHE_SIZE, and re_cache_idx being unsigned, it cannot have negative value.) I'd vote for the exact equality tests.

*/
const re_compiled_code_t *
static uint16_t
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uint32_t

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see now that re_cache_idx is uint16_t. If we want to force a small number we could use uint8_t. Anyway I am even happy with uint32_t as well.
Otherwise LGTM.

@LaszloLango LaszloLango force-pushed the improve-regexp-cache branch from f58c816 to 74cdc68 Compare March 10, 2016 08:47
@LaszloLango
Copy link
Contributor Author

@zherczeg, updated. May I land this?

@LaszloLango LaszloLango force-pushed the improve-regexp-cache branch from 74cdc68 to ba4e52d Compare March 10, 2016 09:28
@zherczeg
Copy link
Member

LGTM

Added eviction mechanism to RegExp cache and small
refactoring. Fixed a bug when logging is enabled.

Related issue: jerryscript-project#927

JerryScript-DCO-1.0-Signed-off-by: László Langó llango.u-szeged@partner.samsung.com
@LaszloLango LaszloLango force-pushed the improve-regexp-cache branch from ba4e52d to 6f536c7 Compare March 10, 2016 12:09
@LaszloLango LaszloLango merged commit 6f536c7 into jerryscript-project:master Mar 10, 2016
@LaszloLango LaszloLango deleted the improve-regexp-cache branch April 1, 2016 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ecma builtins Related to ECMA built-in routines enhancement An improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants