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

RFE: speed up resolving syscalls by name by using hash map #156

Closed
wants to merge 3 commits into from
Closed

RFE: speed up resolving syscalls by name by using hash map #156

wants to merge 3 commits into from

Conversation

varqox
Copy link

@varqox varqox commented May 2, 2019

It slightly mitigates #153.

@pcmoore pcmoore changed the title Sped up resolving syscalls by name by using hash map RFE: speed up resolving syscalls by name by using hash map May 2, 2019
@coveralls
Copy link

coveralls commented May 2, 2019

Coverage Status

Coverage decreased (-2.7%) to 89.358% when pulling a272efe on varqox:master into 3570b5c on seccomp:master.

@pcmoore
Copy link
Member

pcmoore commented Nov 14, 2019

I'm not entirely sure how I feel about this approach at the moment.

If I understand you correctly @varqox, the main motivation for this was issue #153, which I believe we've addressed in the master branch (scheduled for the v2.5 release). How do you feel about this PR now?

@varqox
Copy link
Author

varqox commented Nov 14, 2019

Yes, I did this because before the fix from #180 resolving the name of a syscall was the primary thing that libseccomp did (based on perf analysis). Now it is only roughly 10% of what libseccomp is doing.
Moreover, my solution reduces name resolving only by about 50%. Also, it is not a clean implementation because of the lack of compile-time data manipulation (building the hash map) in C.

For now, I don't think that this solution is worth ~5% performance increase of libseccomp.

@pcmoore
Copy link
Member

pcmoore commented Nov 14, 2019

For now, I don't think that this solution is worth ~5% performance increase of libseccomp.

That is my thinking at the moment too. While there are definitely improvements to be had with respect to syscall name/number resolution, I'm not sure this is the right fix (although a hash map is likely a good idea).

With the conversation above in mind, I'm going to close this PR. Thanks again for your patience and help in resolving the #153.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants