Skip to content

Commit

Permalink
[Java,C#,C++] Allow custom flag/properties to enable precedence checks.
Browse files Browse the repository at this point in the history
Some users would like the ability to enable precedence checks on some
schemas but not others.

Initially, I considered generating flags or system properties with the
schema namespace; however, this forces the more-complex scheme on all
users. It also might get unwieldly with long namespaces. It also assumes
a one-to-one correspondence between namespaces and schemas.

Instead, I have introduced two new system properties that can be applied
at code generation time:

- `sbe.precedence.checks.flagName`, which controls the symbol/macro used
to enable precedence checks at build time in the generated C#/C++ code.
It defaults to `"SBE_ENABLE_PRECEDENCE_CHECKS"`.

- `sbe.precedence.checks.propName`, which controls the property name
used to enable precedence checks at runtime in the generated Java code.
It defaults to `"sbe.enable.precedence.checks"`.

Now, users can run SbeTool with different values for these system
properties to generate code with different enablement flags.

Note above that the defaults are:

- `SBE_ENABLE_PRECEDENCE_CHECKS` rather than
`SBE_ENABLE_SEQUENCING_CHECKS`, and
- `sbe.enable.precedence.checks` rather than
`sbe.enable.sequencing.checks`.

These changes and a change to another system property:

- `sbe.generate.sequencing.checks` -> `sbe.generate.precedence.checks`

address some feedback from Martin on the associated PR #948.
  • Loading branch information
ZachBray committed Nov 13, 2023
1 parent 69d13b6 commit 11afc92
Show file tree
Hide file tree
Showing 10 changed files with 222 additions and 195 deletions.
17 changes: 12 additions & 5 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,9 @@ subprojects {
}

javaLauncher.set(toolchainLauncher)

systemProperty 'sbe.enable.ir.precedence.checks', 'true'
systemProperty 'sbe.enable.test.precedence.checks', 'true'
}
}

Expand Down Expand Up @@ -280,7 +283,8 @@ project(':sbe-tool') {
'sbe.target.language': 'Java',
'sbe.validation.stop.on.error': 'true',
'sbe.validation.xsd': validationXsdPath,
'sbe.generate.sequencing.checks': 'true')
'sbe.generate.precedence.checks': 'true',
'sbe.precedence.checks.propName': 'sbe.enable.test.precedence.checks')
args = ['src/test/resources/json-printer-test-schema.xml',
'src/test/resources/composite-elements-schema.xml',
'src/test/resources/field-order-check-schema.xml']
Expand Down Expand Up @@ -544,7 +548,7 @@ project(':sbe-benchmarks') {
'sbe.validation.xsd': validationXsdPath,
'sbe.java.encoding.buffer.type': 'org.agrona.concurrent.UnsafeBuffer',
'sbe.java.decoding.buffer.type': 'org.agrona.concurrent.UnsafeBuffer',
'sbe.generate.sequencing.checks': 'false')
'sbe.generate.precedence.checks': 'false')
args = ['src/main/resources/car.xml', 'src/main/resources/fix-message-samples.xml']
}

Expand Down Expand Up @@ -734,7 +738,7 @@ tasks.register('generateCSharpCodecsTests', JavaExec) {
'sbe.target.language': 'uk.co.real_logic.sbe.generation.csharp.CSharp',
'sbe.xinclude.aware': 'true',
'sbe.validation.xsd': validationXsdPath,
'sbe.generate.sequencing.checks': 'true')
'sbe.generate.precedence.checks': 'true')
args = ['sbe-tool/src/test/resources/FixBinary.xml',
'sbe-tool/src/test/resources/issue435.xml',
'sbe-tool/src/test/resources/issue483.xml',
Expand All @@ -759,7 +763,9 @@ tasks.register('generateJavaIrCodecs', JavaExec) {
'sbe.output.dir': 'sbe-tool/src/main/java',
'sbe.target.language': 'Java',
'sbe.validation.xsd': validationXsdPath,
'sbe.generate.sequencing.checks': 'true')
'sbe.generate.precedence.checks': 'true',
'sbe.precedence.checks.flagName': 'SBE_ENABLE_IR_PRECEDENCE_CHECKS',
'sbe.precedence.checks.propName': 'sbe.enable.ir.precedence.checks')
args = ['sbe-tool/src/main/resources/sbe-ir.xml']
}

Expand All @@ -770,7 +776,8 @@ tasks.register('generateCppIrCodecs', JavaExec) {
'sbe.output.dir': 'sbe-tool/src/main/cpp',
'sbe.target.language': 'cpp',
'sbe.validation.xsd': validationXsdPath,
'sbe.generate.sequencing.checks': 'true')
'sbe.generate.precedence.checks': 'true',
'sbe.precedence.checks.flagName': 'SBE_ENABLE_IR_PRECEDENCE_CHECKS')
args = ['sbe-tool/src/main/resources/sbe-ir.xml']
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,26 @@
// Therefore, we allow lambdas with code blocks even when a lambda expression is possible.
public final class AccessOrderModel
{
/**
* Whether to generate access order checks.
*/
private static final boolean GENERATE_ACCESS_ORDER_CHECKS = Boolean.parseBoolean(
System.getProperty("sbe.generate.sequencing.checks", "false"));
System.getProperty("sbe.generate.precedence.checks", "false"));

/**
* The name of the symbol or macro that enables access order checks when building
* generated C# or C++ code.
*/
public static final String PRECEDENCE_CHECKS_FLAG_NAME =
System.getProperty("sbe.precedence.checks.flagName", "SBE_ENABLE_PRECEDENCE_CHECKS");

/**
* The name of the system property that enables access order checks at runtime
* in generated Java code.
*/
public static final String PRECEDENCE_CHECKS_PROP_NAME =
System.getProperty("sbe.precedence.checks.propName", "sbe.enable.precedence.checks");

private final Map<Token, String> groupPathsByField = new HashMap<>();
private final Set<Token> topLevelBlockFields = new HashSet<>();
private final CodecInteraction.HashConsingFactory interactionFactory =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ private static CharSequence generateFullyEncodedCheck(final AccessOrderModel acc

sb.append(indent).append("void checkEncodingIsComplete()\n")
.append(indent).append("{\n")
.append("#if defined(SBE_ENABLE_SEQUENCING_CHECKS)\n")
.append("#if defined(").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")\n")
.append(indent).append(INDENT).append("switch (m_codecState)\n")
.append(indent).append(INDENT).append("{\n");

Expand Down Expand Up @@ -321,7 +321,7 @@ private static CharSequence generateAccessOrderListenerCall(
}

final StringBuilder sb = new StringBuilder();
sb.append("#if defined(SBE_ENABLE_SEQUENCING_CHECKS)\n")
sb.append("#if defined(").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")\n")
.append(indent).append(methodName).append("(");

for (int i = 0; i < arguments.length; i++)
Expand Down Expand Up @@ -3083,7 +3083,7 @@ private CharSequence generateDecoderWrapListenerCall(final AccessOrderModel acce
if (accessOrderModel.versionCount() == 1)
{
final StringBuilder sb = new StringBuilder();
sb.append("#if defined(SBE_ENABLE_SEQUENCING_CHECKS)\n")
sb.append("#if defined(").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")\n")
.append(TWO_INDENT).append("codecState(")
.append(qualifiedStateCase(accessOrderModel.latestVersionWrappedState())).append(");\n")
.append("#endif\n");
Expand All @@ -3101,7 +3101,7 @@ private CharSequence generateEncoderWrapListener(final AccessOrderModel accessOr
}

final StringBuilder sb = new StringBuilder();
sb.append("#if defined(SBE_ENABLE_SEQUENCING_CHECKS)\n")
sb.append("#if defined(").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")\n")
.append(TWO_INDENT).append("codecState(")
.append(qualifiedStateCase(accessOrderModel.latestVersionWrappedState()))
.append(");\n")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1583,7 +1583,7 @@ private static CharSequence generateFullyEncodedCheck(

sb.append(indent).append("public void CheckEncodingIsComplete()\n")
.append(indent).append("{\n")
.append("#if SBE_ENABLE_SEQUENCING_CHECKS\n")
.append("#if ").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append("\n")
.append(indent).append(INDENT).append("switch (_codecState)\n")
.append(indent).append(INDENT).append("{\n");

Expand Down Expand Up @@ -1670,7 +1670,7 @@ private static CharSequence generateAccessOrderListenerCall(
}

final StringBuilder sb = new StringBuilder();
sb.append("#if SBE_ENABLE_SEQUENCING_CHECKS\n")
sb.append("#if ").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append("\n")
.append(indent).append(methodName).append("(");

for (int i = 0; i < arguments.length; i++)
Expand Down Expand Up @@ -1936,7 +1936,7 @@ private CharSequence generateEncoderWrapListener(
}

final StringBuilder sb = new StringBuilder();
sb.append("#if SBE_ENABLE_SEQUENCING_CHECKS\n")
sb.append("#if ").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append("\n")
.append(indent).append("codecState(")
.append(qualifiedStateCase(accessOrderModel.latestVersionWrappedState()))
.append(");\n")
Expand Down Expand Up @@ -2556,7 +2556,7 @@ private CharSequence generateDisplay(
append(sb, TWO_INDENT, " int originalLimit = this.Limit;");
if (null != accessOrderModel)
{
sb.append("#if SBE_ENABLE_SEQUENCING_CHECKS\n");
sb.append("#if ").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append("\n");
append(sb, TWO_INDENT, " CodecState originalState = _codecState;");
sb.append(THREE_INDENT).append("_codecState = ")
.append(qualifiedStateCase(accessOrderModel.notWrappedState())).append(";\n");
Expand Down Expand Up @@ -2588,7 +2588,7 @@ private CharSequence generateDisplay(
sb.append('\n');
if (null != accessOrderModel)
{
sb.append("#if SBE_ENABLE_SEQUENCING_CHECKS\n");
sb.append("#if ").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append("\n");
append(sb, TWO_INDENT, " _codecState = originalState;");
sb.append("#endif\n");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,10 @@ private static CharSequence generateFieldOrderStates(final AccessOrderModel acce

sb.append(" private static final boolean ENABLE_BOUNDS_CHECKS = ")
.append("!Boolean.getBoolean(\"agrona.disable.bounds.checks\");\n\n");
sb.append(" private static final boolean SBE_ENABLE_SEQUENCING_CHECKS = ")
sb.append(" private static final boolean ")
.append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(" = ")
.append("Boolean.parseBoolean(System.getProperty(\n")
.append(" \"sbe.enable.sequencing.checks\",\n")
.append(" \"").append(AccessOrderModel.PRECEDENCE_CHECKS_PROP_NAME).append("\",\n")
.append(" Boolean.toString(ENABLE_BOUNDS_CHECKS)));\n\n");

sb.append(" /**\n");
Expand Down Expand Up @@ -410,7 +411,7 @@ private static void generateFullyEncodedCheck(

sb.append(" public void checkEncodingIsComplete()\n")
.append(" {\n")
.append(" if (SBE_ENABLE_SEQUENCING_CHECKS)\n")
.append(" if (").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")\n")
.append(" {\n")
.append(" switch (codecState)\n")
.append(" {\n");
Expand Down Expand Up @@ -493,7 +494,7 @@ private static CharSequence generateAccessOrderListenerCall(
}

final StringBuilder sb = new StringBuilder();
sb.append(indent).append("if (SBE_ENABLE_SEQUENCING_CHECKS)\n")
sb.append(indent).append("if (").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")\n")
.append(indent).append("{\n")
.append(indent).append(" ").append(methodName).append("(");

Expand Down Expand Up @@ -755,7 +756,7 @@ private CharSequence generateEncoderWrapListener(
}

final StringBuilder sb = new StringBuilder();
sb.append(indent).append("if (SBE_ENABLE_SEQUENCING_CHECKS)")
sb.append(indent).append("if (").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")")
.append("\n").append(indent).append("{\n")
.append(indent).append(" codecState(")
.append(qualifiedStateCase(accessOrderModel.latestVersionWrappedState()))
Expand Down Expand Up @@ -3424,7 +3425,7 @@ private CharSequence generateDecoderFlyweightCode(

if (null != accessOrderModel)
{
methods.append(" if (SBE_ENABLE_SEQUENCING_CHECKS)\n")
methods.append(" if (").append(AccessOrderModel.PRECEDENCE_CHECKS_FLAG_NAME).append(")\n")
.append(" {\n")
.append(" codecState(currentCodecState);\n")
.append(" }\n\n");
Expand Down
Loading

0 comments on commit 11afc92

Please sign in to comment.