You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
The text was updated successfully, but these errors were encountered:
In Pull Request #22, some comments strongly suggest usage as a dynamic library isn't supported:
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.
The text was updated successfully, but these errors were encountered: