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

[C++] Unnecessary overhead added with precedence checks #1032

Closed
szymonwieloch opened this issue Dec 9, 2024 · 2 comments
Closed

[C++] Unnecessary overhead added with precedence checks #1032

szymonwieloch opened this issue Dec 9, 2024 · 2 comments

Comments

@szymonwieloch
Copy link

When you enable precedence checks (sbe.enable.precedence.checks=true), the generated code is supposed to be managed using the SBE_ENABLE_PRECEDENCE_CHECKS macro. The expectation is that with this flag enabled, precedence check should be enabled and with this flag NOT enabled there should be no overhead and the code should be usable in production. However upon inspection it seems that the code does have some overhead also when the macro is disabled. For example the m_codeState and corresponding pointers in the group wrappers do add some overhead. Currently user has three different options of code generation:

  1. sbe.enable.precedence.checks=false - the "standard" version without any overhead and no checks
  2. sbe.enable.precedence.checks=true and the SBE_ENABLE_PRECEDENCE_CHECKS macro enabled - debug only version with checks and significant overhead
  3. sbe.enable.precedence.checks=true and the SBE_ENABLE_PRECEDENCE_CHECKS macro disabled - quite a pointless version with zero checks but some overhead caused by adding more variables.

I can see two ways to solve this situation - either drop the SBE_ENABLE_PRECEDENCE_CHECKS macro and with the sbe.enable.precedence.checks=true flag generate code that always does precedence checks (should simplify the generated code) or polish the generated code not to have any overhead with the SBE_ENABLE_PRECEDENCE_CHECKS macro enabled. The second approach seems more complicated but also much more consistent with what other language generators do.

@nbradac
Copy link
Contributor

nbradac commented Dec 13, 2024

This PR (#1036) has been merged to address this issue. The m_codecStatePtr won't exist when SBE_ENABLE_PRECEDENCE_CHECKS isn't set.

Are there other places where the precedence checks are adding overhead that you (@szymonwieloch) can see?

@szymonwieloch
Copy link
Author

Hi. Thank you for fixing it. For now I can't see anything else that requires fixing.

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

2 participants