Directly free structure managed by imemo tmpbuf #8493
Merged
Conversation
This file contains 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
nobu
reviewed
Sep 21, 2023
parse.y
Outdated
Comment on lines
12725
to
12730
| VALUE tmpbuf = rb_imemo_tmpbuf_auto_free_pointer(); | ||
| struct rb_args_info *args = ZALLOC(struct rb_args_info); | ||
| rb_imemo_tmpbuf_set_ptr(tmpbuf, args); | ||
| args->imemo = tmpbuf; | ||
| node = NEW_NODE(NODE_ARGS, 0, 0, args, &NULL_LOC); | ||
| RB_OBJ_WRITTEN(p->ast, Qnil, tmpbuf); |
There was a problem hiding this comment.
If no longer using imemo, NODE should be created before ZALLOC to store it.
Otherwise that memory would leak if NEW_NODE failed.
There was a problem hiding this comment.
Thanks for the review. Changed the order of allocations.
NODE_ARGS, NODE_ARYPTN, NODE_FNDPTN manage memory of their structure by imemo tmpbuf Object. However rb_ast_struct has reference to NODE. Then these memory can be freed directly when rb_ast_struct is freed. This commit reduces parser's dependency on CRuby functions.
yui-knk
force-pushed
the
remove_rb_imemo_tmpbuf_auto_free_pointer
branch
from
1e7d599
to
e8efbdf
Compare
|
@nobu Can I ask you to check this PR again? |
nobu
approved these changes
Sep 22, 2023
|
Thank you! |
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.
NODE_ARGS, NODE_ARYPTN, NODE_FNDPTN manage memory of their structure by imemo tmpbuf Object.
However rb_ast_struct has reference to NODE. Then these memory can be freed directly when rb_ast_struct is freed.
This commit reduces parser's dependency on CRuby functions.