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.

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

Jump to bottom

Multithread string search #2665

Merged
merged 3 commits into from
Jun 27, 2022
Merged

Multithread string search #2665

merged 3 commits into from
Jun 27, 2022

Conversation

wargio
Copy link
Member

@wargio wargio commented Jun 6, 2022 •
edited
Loading

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
  • I've added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the rizin book with the relevant information (if needed)

Detailed description

The following changes to the code has been applied:

  • Added threads to the string search for a faster loading time (izz on 600 Mb file uses less ram and takes with 4-threads 0m28.528s compared to single-thread 0m42.386s).
  • Fixed usage of bin.maxstrbuf with a proper warning message.
  • Removed HtUP *strings_db; from RzBinObject since it is used only when rz_bin_file_strings is called and nowhere else.
  • Refactored search_str.c and bfile.c for thread-safety and correctness.

@wargio wargio added this to the 0.5.0 milestone Jun 6, 2022
@wargio wargio force-pushed the multithread-string-search branch from 3920cb3 to e2ff82f Compare June 6, 2022 14:11
@wargio wargio changed the title Multithread string search Multithread string search + refactoring Jun 6, 2022
@wargio wargio force-pushed the multithread-string-search branch 2 times, most recently from 60c9fd1 to a6e1155 Compare June 8, 2022 22:44
@wargio wargio force-pushed the multithread-string-search branch 2 times, most recently from 05fc7ac to de630ae Compare June 10, 2022 17:55
@wargio wargio force-pushed the multithread-string-search branch from 113394b to dc10818 Compare June 20, 2022 18:49
@github-actions github-actions bot removed the RzSocket label Jun 20, 2022
@wargio wargio force-pushed the multithread-string-search branch from 835c84d to d654d47 Compare June 21, 2022 19:08
@wargio wargio marked this pull request as ready for review June 21, 2022 19:22
@wargio wargio changed the title Multithread string search + refactoring Multithread string search Jun 21, 2022
XVilka
XVilka requested changes Jun 22, 2022
View reviewed changes
Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

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

Looks great! And definitely faster

librz/bin/bfile_string.c Outdated Show resolved Hide resolved
librz/bin/bfile_string.c Outdated Show resolved Hide resolved
librz/bin/bfile_string.c Outdated Show resolved Hide resolved
librz/bin/bfile_string.c Outdated Show resolved Hide resolved
librz/bin/bfile_string.c Outdated Show resolved Hide resolved
librz/bin/bfile_string.c Show resolved Hide resolved
librz/bin/bfile_string.c Outdated Show resolved Hide resolved
librz/bin/bfile_string.c Outdated Show resolved Hide resolved
librz/bin/bfile_string.c Outdated Show resolved Hide resolved
librz/bin/bfile_string.c Outdated Show resolved Hide resolved
@wargio wargio force-pushed the multithread-string-search branch from 53f931c to 98d5bf8 Compare June 23, 2022 16:25
@wargio wargio requested a review from XVilka June 23, 2022 16:25
@wargio wargio force-pushed the multithread-string-search branch from 12225f1 to b9a7fba Compare June 25, 2022 11:13
@wargio wargio merged commit 6567e6b into dev Jun 27, 2022
@wargio wargio deleted the multithread-string-search branch June 27, 2022 21:34
@re-fox re-fox mentioned this pull request Jun 27, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XVilka XVilka XVilka approved these changes

@ret2libc ret2libc Awaiting requested review from ret2libc ret2libc is a code owner

@thestr4ng3r thestr4ng3r Awaiting requested review from thestr4ng3r thestr4ng3r is a code owner

@kazarmy kazarmy Awaiting requested review from kazarmy kazarmy is a code owner

Assignees
No one assigned
Labels
API rz-test RzBin RzUtil
Projects
None yet
Milestone
0.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@wargio @XVilka