Skip to content

[API] Improve the performance of the external magic id search #1506

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.

Sign up for GitHub

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
merged 1 commit into from
Jan 9, 2017
Merged

[API] Improve the performance of the external magic id search #1506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion docs/02.API-REFERENCE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,8 @@ jerry_cleanup (void);

Registers an external magic string array.

*Note*: The strings in the array must be sorted by size at first, then lexicographically.

**Prototype**

```c
Expand All @@ -275,11 +277,12 @@ jerry_register_magic_strings (const jerry_char_ptr_t *ex_str_items_p,
jerry_init (JERRY_INIT_EMPTY);

// must be static, because 'jerry_register_magic_strings' does not copy
// the items must be sorted by size at first, then lexicographically
static const jerry_char_ptr_t magic_string_items[] = {
(const jerry_char_ptr_t) "magicstring1",
(const jerry_char_ptr_t) "magicstring2",
(const jerry_char_ptr_t) "magicstring3"
};
};
uint32_t num_magic_string_items = (uint32_t) (sizeof (magic_string_items) / sizeof (jerry_char_ptr_t));

// must be static, because 'jerry_register_magic_strings' does not copy
Expand Down
116 changes: 97 additions & 19 deletions jerry-core/lit/lit-magic-strings.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,24 @@ lit_magic_strings_ex_set (const lit_utf8_byte_t **ex_str_items, /**< character a
lit_utf8_size_t string_size = lit_zt_utf8_string_size (lit_get_magic_string_ex_utf8 (id));
JERRY_ASSERT (JERRY_CONTEXT (lit_magic_string_ex_sizes)[id] == string_size);
JERRY_ASSERT (JERRY_CONTEXT (lit_magic_string_ex_sizes)[id] <= LIT_MAGIC_STRING_LENGTH_LIMIT);

/**
* Check whether the strings are sorted by size and lexicographically,
* e.g., "Bb" < "aa" < "aaa" < "xyz0".
*/
if (id > 0)
{
const lit_magic_string_ex_id_t prev_id = id - 1;
const lit_utf8_size_t prev_string_size = lit_get_magic_string_ex_size (prev_id);
JERRY_ASSERT (prev_string_size <= string_size);

if (prev_string_size == string_size)
{
const lit_utf8_byte_t *prev_ex_string_p = lit_get_magic_string_ex_utf8 (prev_id);
const lit_utf8_byte_t *curr_ex_string_p = lit_get_magic_string_ex_utf8 (id);
JERRY_ASSERT (memcmp (prev_ex_string_p, curr_ex_string_p, string_size) < 0);
}
}
}
#endif /* !JERRY_NDEBUG */
} /* lit_magic_strings_ex_set */
Expand Down Expand Up @@ -264,22 +282,51 @@ lit_magic_string_ex_id_t
lit_is_ex_utf8_string_magic (const lit_utf8_byte_t *string_p, /**< utf-8 string */
lit_utf8_size_t string_size) /**< string size in bytes */
{
/* TODO: Improve performance of search */
const uint32_t magic_string_ex_count = lit_get_magic_string_ex_count ();

for (lit_magic_string_ex_id_t id = (lit_magic_string_ex_id_t) 0;
id < JERRY_CONTEXT (lit_magic_string_ex_count);
id = (lit_magic_string_ex_id_t) (id + 1))
if (magic_string_ex_count == 0
|| string_size > lit_get_magic_string_ex_size (magic_string_ex_count - 1))
{
return (lit_magic_string_ex_id_t) magic_string_ex_count;
}

lit_magic_string_ex_id_t first = 0;
lit_magic_string_ex_id_t last = (lit_magic_string_ex_id_t) magic_string_ex_count;

while (first < last)
{
if (string_size == lit_get_magic_string_ex_size (id))
const lit_magic_string_ex_id_t middle = (first + last) / 2;
const lit_utf8_byte_t *ext_string_p = lit_get_magic_string_ex_utf8 (middle);
const lit_utf8_size_t ext_string_size = lit_get_magic_string_ex_size (middle);

if (string_size == ext_string_size)
{
if (memcmp (string_p, lit_get_magic_string_ex_utf8 (id), string_size) == 0)
const int string_compare = memcmp (ext_string_p, string_p, string_size);

if (string_compare == 0)
{
return middle;
}
else if (string_compare < 0)
{
return id;
first = middle + 1;
}
else
{
last = middle;
}
}
else if (string_size > ext_string_size)
{
first = middle + 1;
}
else
{
last = middle;
}
}

return JERRY_CONTEXT (lit_magic_string_ex_count);
return (lit_magic_string_ex_id_t) magic_string_ex_count;
} /* lit_is_ex_utf8_string_magic */

/**
Expand All @@ -294,26 +341,57 @@ lit_is_ex_utf8_string_pair_magic (const lit_utf8_byte_t *string1_p, /**< first u
const lit_utf8_byte_t *string2_p, /**< second utf-8 string */
lit_utf8_size_t string2_size) /**< second string size in bytes */
{
/* TODO: Improve performance of search */
lit_utf8_size_t total_string_size = string1_size + string2_size;
const uint32_t magic_string_ex_count = lit_get_magic_string_ex_count ();
const lit_utf8_size_t total_string_size = string1_size + string2_size;

for (lit_magic_string_ex_id_t id = (lit_magic_string_ex_id_t) 0;
id < JERRY_CONTEXT (lit_magic_string_ex_count);
id = (lit_magic_string_ex_id_t) (id + 1))
if (magic_string_ex_count == 0
|| total_string_size > lit_get_magic_string_ex_size (magic_string_ex_count - 1))
{
if (total_string_size == lit_get_magic_string_ex_size (id))
return (lit_magic_string_ex_id_t) magic_string_ex_count;
}

lit_magic_string_ex_id_t first = 0;
lit_magic_string_ex_id_t last = (lit_magic_string_ex_id_t) magic_string_ex_count;

while (first < last)
{
const lit_magic_string_ex_id_t middle = (first + last) / 2;
const lit_utf8_byte_t *ext_string_p = lit_get_magic_string_ex_utf8 (middle);
const lit_utf8_size_t ext_string_size = lit_get_magic_string_ex_size (middle);

if (total_string_size == ext_string_size)
{
const lit_utf8_byte_t *ex_magic_string_p = lit_get_magic_string_ex_utf8 (id);
int string_compare = memcmp (ext_string_p, string1_p, string1_size);

if (string_compare == 0)
{
string_compare = memcmp (ext_string_p + string1_size, string2_p, string2_size);
}

if (memcmp (string1_p, ex_magic_string_p, string1_size) == 0
&& memcmp (string2_p, ex_magic_string_p + string1_size, string2_size) == 0)
if (string_compare == 0)
{
return middle;
}
else if (string_compare < 0)
{
return id;
first = middle + 1;
}
else
{
last = middle;
}
}
else if (total_string_size > ext_string_size)
{
first = middle + 1;
}
else
{
last = middle;
}
}

return JERRY_CONTEXT (lit_magic_string_ex_count);
return (lit_magic_string_ex_id_t) magic_string_ex_count;
} /* lit_is_ex_utf8_string_pair_magic */

/**
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test-api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ handler_construct (const jerry_value_t func_obj_val, /**< function object */
*/
#define JERRY_MAGIC_STRING_ITEMS \
JERRY_MAGIC_STRING_DEF (GLOBAL, global) \
Copy link
Contributor

@robertsipka robertsipka Jan 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The global is already in the list of ECMA magic strings, so it doesn't have to be an external magic string because the defined magic string id will be found at first. But I think it could be removed in a follow-up patch, otherwise LGTM (informally).

JERRY_MAGIC_STRING_DEF (CONSOLE, console) \
JERRY_MAGIC_STRING_DEF (GREEK_ZERO_SIGN, \xed\xa0\x80\xed\xb6\x8a)
JERRY_MAGIC_STRING_DEF (GREEK_ZERO_SIGN, \xed\xa0\x80\xed\xb6\x8a) \
JERRY_MAGIC_STRING_DEF (CONSOLE, console)

#define JERRY_MAGIC_STRING_DEF(NAME, STRING) \
static const char jerry_magic_string_ex_ ## NAME[] = # STRING;
Expand Down