Skip to content

Moving code in macro arguments out of macros#6432

Merged
matz merged 1 commit into
mruby:masterfrom
dearblue:hash
Nov 22, 2024
Merged

Moving code in macro arguments out of macros#6432
matz merged 1 commit into
mruby:masterfrom
dearblue:hash

Conversation

@dearblue

Copy link
Copy Markdown
Contributor

In src/hash.c, there are code blocks that are passed as macro arguments. These code blocks are interpreted as part of the macro function, so breakpoints cannot be set in the debugger. Also, the gcov command will aggregate them to the caller, and the code in the block will not be counted.

This patch will prevent them from being interpreted as part of a macro, and thus the aforementioned problems will no longer occur.


This patch has a performance impact.
Checked with valgrind --tool=cachegrind, bin/mrbtest increases the number of instruction cycles by 0.4 %, and the microbenchmark shown at #6414 (comment) 2478972123 increases the number of instruction cycles by about 10 %.

@dearblue dearblue requested a review from matz as a code owner November 20, 2024 14:51
@github-actions github-actions Bot added the core label Nov 20, 2024
@matz

matz commented Nov 21, 2024

Copy link
Copy Markdown
Member

Recent commit (inlining h_set_without_ib_adjustment) caused conflict. I will merge this once you have resolved the conflict.

In `src/hash.c`, there are code blocks that are passed as macro arguments.
These code blocks are interpreted as part of the macro function, so breakpoints cannot be set in the debugger.
Also, the gcov command will aggregate them to the caller, and the code in the block will not be counted.

This patch will prevent them from being interpreted as part of a macro, and thus the aforementioned problems will no longer occur.
@dearblue

Copy link
Copy Markdown
Contributor Author

The change is to remove the prototype declaration by moving the definition position of entry_skip_deleted().

The prototype declaration of ib_it_entry() added since the last time is necessary for the convenience of the definition position of the newly added ib_it_find_by_key().
Moving ib_it_find_by_key() makes the prototype declaration unnecessary, but I think it would be better to be closer to ib_it_init().

@matz matz merged commit 781359a into mruby:master Nov 22, 2024
@matz

matz commented Nov 22, 2024

Copy link
Copy Markdown
Member

Thank you!

dearblue added a commit to dearblue/mruby that referenced this pull request Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants