Skip to content
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

Reduce call to strlen when possible #5192

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lygstate
Copy link
Contributor

@lygstate lygstate commented Dec 9, 2024

Closed: #4978
Closed: #4979

  • Improve jerry_string_sz only accept UTF-8 string(that's a rare case accept CESU-8 in c/cpp code)
  • The document about jerry_string_sz only support ASCII(the fact is CESU-8 before this MR), update it to support UTF-8 after this MR
  • Improve all _sz function to take jerry_value_t that can construct from jerry_string_sz
  • Improve JERRY_ZSTR_ARG can only accept string literal(that's UTF-8)
  • Add function jerry_value_list_free to free a list of jerry_value_t
  • All call to jerry_string/jerry_string_cesu8 in core indeed are removed, so when there is no linkage to it, code size is saved

The prototype of new/improved function/macros is:

jerry_value_t jerry_string_cesu8 (const jerry_char_t *buffer_p, jerry_size_t buffer_size);
jerry_value_t jerry_string_utf8 (const jerry_char_t *buffer_p, jerry_size_t buffer_size);
#define jerry_string_sz(str) jerry_string_utf8 (JERRY_ZSTR_ARG (str))

jerry_value_t jerry_error_sz (jerry_error_t error_type, const jerry_value_t message_sz);
jerry_value_t jerry_regexp_sz (const jerry_value_t pattern_sz, uint16_t flags);
jerry_value_t jerry_object_delete_sz (const jerry_value_t object, const jerry_value_t key_sz);
jerry_value_t jerry_object_get_sz (const jerry_value_t object, const jerry_value_t key_sz);
jerry_value_t jerry_object_has_sz (const jerry_value_t object, const jerry_value_t key_sz);
jerry_value_t jerry_object_set_sz (jerry_value_t object, const jerry_value_t key_sz, const jerry_value_t value);

Rename jerry_port_log to jerry_port_log_buffer

Add buffer_size parameter for function jerry_port_log_buffer, so that jerry_port_log_buffer
won't need calculate strlen when printing

JerryScript-DCO-1.0-Signed-off-by: Yonggang Luo [email protected]

@zherczeg
Copy link
Member

zherczeg commented Dec 9, 2024

What about keeping jerry_string_sz? Maybe adding jerry_string_utf8_sz()? I suspect cesu8/ascii is the same since ascii is 7 bit.

@lygstate
Copy link
Contributor Author

lygstate commented Dec 9, 2024

What about keeping jerry_string_sz? Maybe adding jerry_string_utf8_sz()? I suspect cesu8/ascii is the same since ascii is 7 bit.

jerry_string_sz is equal to jerry_sz_cesu8 and jerry_string_cesu8 before this MR,
we have jerry_sz_utf8 and jerry_string_utf8 that have the feature of jerry_string_utf8_sz after this MR

jerry_sz_cesu8 is a macro for literal,
jerry_string_cesu8 is a function for ptr,length pair

@lygstate
Copy link
Contributor Author

lygstate commented Dec 9, 2024

I suspect cesu8/ascii is the same since ascii is 7 bit.
I have the willing to improve string #5182

So that refactoring ASCII out is a thing because for most/(maybe all) internal string is ASCII, and for ASCII the check function is much simpler.

The macro give us future optimize string literal store, as jerry_sz_ascii declered string can be in literal store

@zherczeg
Copy link
Member

zherczeg commented Dec 9, 2024

Cesu8 is as efficient as ascii for checking, since the cpu branch predictor learns the simplest case. Two functions actually increase code size.

@lygstate
Copy link
Contributor Author

lygstate commented Dec 9, 2024

Cesu8 is as efficient as ascii for checking, since the cpu branch predictor learns the simplest case. Two functions actually increase code size.

OK, I do the following to achieve the same effect, to code changes reduced a lot

  • Improve jerry_string_sz only accept UTF-8 string(that's a rare case accept CESU-8 in c/cpp code)
  • The document about jerry_string_sz only support ASCII(the fact is CESU-8 before this MR), update it to support UTF-8 after this MR

In summary jerry_string_sz preserved

@lygstate lygstate force-pushed the strlen-reduced-all branch 10 times, most recently from e7a63f4 to 215c086 Compare December 12, 2024 18:17
@lygstate lygstate force-pushed the strlen-reduced-all branch 2 times, most recently from 80dba58 to 466caea Compare December 17, 2024 10:29
@lygstate lygstate mentioned this pull request Dec 17, 2024
* Improve jerry_string_sz only accept UTF-8 string(that's a rare case accept CESU-8 in c/cpp code)
* The document about jerry_string_sz only support ASCII(the fact is CESU-8 before this MR), update it to support UTF-8 after this MR
* Improve all _sz function to take jerry_value_t that can construct from `jerry_string_sz`
* Improve JERRY_ZSTR_ARG can only accept string literal(that's UTF-8)
* Add function jerry_value_list_free to free a list of jerry_value_t
* All call to jerry_string/jerry_string_cesu8 in core indeed are removed, so when there is no linkage to it, code size is saved

The prototype of new/improved function/macros is:
```c
jerry_value_t jerry_string_cesu8 (const jerry_char_t *buffer_p, jerry_size_t buffer_size);
jerry_value_t jerry_string_utf8 (const jerry_char_t *buffer_p, jerry_size_t buffer_size);
#define jerry_string_sz(str) jerry_string_utf8 (JERRY_ZSTR_ARG (str))

jerry_value_t jerry_error_sz (jerry_error_t error_type, const jerry_value_t message_sz);
jerry_value_t jerry_throw_sz (jerry_error_t error_type, const jerry_value_t message_sz);
jerry_value_t jerry_regexp_sz (const jerry_value_t pattern_sz, uint16_t flags);
jerry_value_t jerry_object_delete_sz (const jerry_value_t object, const jerry_value_t key_sz);
jerry_value_t jerry_object_get_sz (const jerry_value_t object, const jerry_value_t key_sz);
jerry_value_t jerry_object_has_sz (const jerry_value_t object, const jerry_value_t key_sz);
jerry_value_t jerry_object_set_sz (jerry_value_t object, const jerry_value_t key_sz, const jerry_value_t value);
```

JerryScript-DCO-1.0-Signed-off-by: Yonggang Luo [email protected]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants