String handling improvements #331
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Also depends on #332 |
|
@akiss77, please rebase and squash |
* iotjs::ReadFile: Read file contents in one go instead of doing it in a loop. * iotjs::JObject: Remove String-based method variants. Those variants are completely unused in the project. All Error constructions and property accesses are char*-based. (Should they be needed in the future, then a better implementation is suggested: the now-removed String-based methods always fell back to the char*-based variant by simply passing on the memory buffer of the String. However, they could have their own clone of the function body invoking _sz jerry API variants and passing the size of the String, so that length re-computation does not happen.) * iotjs::JObject::JObject(const String&): The jerry API has a create_string_sz variant that takes size as a second parameter, if already known. Since every String knows its own size, we can make use of it. This also implies that we don't need to assert that the String has memory allocated, since create_string_sz can deal with NULL if size is 0. * iotjs::JObject::GetString(): When a String is contructed just to be filled in later, there is no need to be constructed with the empty "" string; NULL is enough as source data, memory will be allocated anyway for the specified size. * iotjs::String copy constructor: Add clarification comment. * iotjs_module_buffer.cpp/___ToString_native(): Create String right away with buffer data. Make sure String capacity is at least one, so that String allocates memory. * iotjs_module_buffer.cpp/___ToString_native(): Precompute length of string in the buffer (look for the terminating zero) and create String with that size. This may even improve memory and performance slightly by not allocating and copying unneeded parts for/of the buffer. * iotjs_module_process.cpp/___CompileNativePtr_native(): The loop scanning through the natives array superfluously replicates the functionality of the standard strcmp. * iotjs_module_httpparser.cpp: JObject can be directly constructed from String, there is no need for taking its raw pointer. * calls to Eval: Prevent re-calculation of string lengths when already known. IoT.js-DCO-1.0-Signed-off-by: Akos Kiss [email protected]
akosthekiss
force-pushed
the
string-impr
branch
from
28eeb0e to
16de549
Compare
|
@LaszloLango done |
| @@ -51,6 +51,9 @@ class String { | |||
| explicit String(const char* data, int size = -1, int cap = -1); | |||
|
|
|||
| // Create string object from other string object. | |||
| // (Actually unimplemented. Declaration of copy constructor is needed for | |||
| // return value optimization to work. However, linker error will be emitted | |||
| // if copy construction is used in any other situation.) | |||
| String(const String& other); | |||
There was a problem hiding this comment.
IMHO this should be private if it is only for RVO.
There was a problem hiding this comment.
I have to fix myself. That would break RVO.
|
@akiss77, thanks. LGTM |
|
LGTM |
pmarcinkiew
pushed a commit
to pmarcinkiew/iotjs
that referenced
this pull request
Aug 29, 2017
remove the CONFIG_CANCELLATION_POINTS conditional at task_setcancelty…
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've reviewed some of the string handling parts of the code base. This PR show the results of my findings spearated into independent patches.
Builds on #330