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

Consider making this a header-only library. #31

Open
jchv opened this issue Sep 8, 2019 · 0 comments
Open

Consider making this a header-only library. #31

jchv opened this issue Sep 8, 2019 · 0 comments

Comments

@jchv
Copy link

jchv commented Sep 8, 2019

In Pull Request #22, some comments strongly suggest usage as a dynamic library isn't supported:

Actually, this is my first big question, what is your rationale to not just include kaitaistruct.* and kaitaistream.* in your project, as this is indeed the recommended way to do it.

The main problem with any dynamically linked library is that it impedes inlining of the simple methods like read_u1, read_s1, etc, which are used very often. Lack of inlining might lead to serious performance problems — we don't have good reproducible benchmarks, but in some tests I've done earlier manually, I've seen 30-50% performance drop.

By making this library header-only, it would actually allow users to install the library without having to worry about these issues. Further, if desired, a single amalgamated header file could be provided that merges all of the headers into one, allowing even easier inclusion into projects.

One downside versus a shared library is the fact that you would need to worry about configuring bits like zlib and iconv manually. However, considering the recommended approach already has this caveat, I think it's worth considering anyways.

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

No branches or pull requests

1 participant