Skip to content

Commit

Permalink
Streamline how we compile if/then/else (#1079)
Browse files Browse the repository at this point in the history 
Signed-off-by: Juan Cruz Viotti <[email protected]>
  • Loading branch information
@jviotti
jviotti authored Aug 31, 2024
1 parent adf7305 commit 5165ffb
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 126 deletions.
19 changes: 5 additions & 14 deletions src/jsonschema/compile_describe.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ struct DescribeVisitor {
return message.str();
}

auto operator()(const SchemaCompilerLogicalTry &) const -> std::string {
auto operator()(const SchemaCompilerLogicalTryMark &) const -> std::string {
assert(this->keyword == "if");
std::ostringstream message;
message << "The " << to_string(this->target.type())
Expand Down Expand Up @@ -266,15 +266,6 @@ struct DescribeVisitor {
}

auto operator()(const SchemaCompilerAnnotationEmit &) const -> std::string {
if (this->keyword == "if") {
assert(this->annotation == JSON{true});
std::ostringstream message;
message
<< "The " << to_string(this->target.type())
<< " value successfully validated against the conditional subschema";
return message.str();
}

if (this->keyword == "properties") {
assert(this->annotation.is_string());
std::ostringstream message;
Expand Down Expand Up @@ -1551,8 +1542,8 @@ struct DescribeVisitor {
return unknown();
}

auto operator()(const SchemaCompilerLogicalWhenNoAdjacentAnnotations &step)
const -> std::string {
auto operator()(const SchemaCompilerLogicalWhenUnmarked &step) const
-> std::string {
if (this->keyword == "else") {
assert(!step.children.empty());
std::ostringstream message;
Expand All @@ -1571,8 +1562,8 @@ struct DescribeVisitor {
return unknown();
}

auto operator()(const SchemaCompilerLogicalWhenAdjacentAnnotations &step)
const -> std::string {
auto
operator()(const SchemaCompilerLogicalWhenMarked &step) const -> std::string {
if (this->keyword == "then") {
assert(!step.children.empty());
std::ostringstream message;
Expand Down
57 changes: 28 additions & 29 deletions src/jsonschema/compile_evaluate.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -721,15 +721,12 @@ auto evaluate_step(
}

CALLBACK_POST("SchemaCompilerLogicalWhenDefines", logical);
} else if (std::holds_alternative<
SchemaCompilerLogicalWhenNoAdjacentAnnotations>(step)) {
SOURCEMETA_TRACE_START(trace_id,
"SchemaCompilerLogicalWhenNoAdjacentAnnotations");
const auto &logical{
std::get<SchemaCompilerLogicalWhenNoAdjacentAnnotations>(step)};
} else if (std::holds_alternative<SchemaCompilerLogicalWhenUnmarked>(step)) {
SOURCEMETA_TRACE_START(trace_id, "SchemaCompilerLogicalWhenUnmarked");
const auto &logical{std::get<SchemaCompilerLogicalWhenUnmarked>(step)};
context.push(logical);
EVALUATE_CONDITION_GUARD("SchemaCompilerLogicalWhenNoAdjacentAnnotations",
logical, instance);
EVALUATE_CONDITION_GUARD("SchemaCompilerLogicalWhenUnmarked", logical,
instance);
const auto &value{context.resolve_value(logical.value, instance)};

// TODO: How can we avoid this expensive pointer manipulation?
Expand All @@ -738,9 +735,8 @@ auto evaluate_step(
expected_evaluate_path.push_back({value});
const auto &current_annotations{context.annotations(
context.instance_location(), expected_evaluate_path)};
EVALUATE_IMPLICIT_PRECONDITION(
"SchemaCompilerLogicalWhenNoAdjacentAnnotations", logical,
current_annotations.empty());
EVALUATE_IMPLICIT_PRECONDITION("SchemaCompilerLogicalWhenUnmarked", logical,
current_annotations.empty());

CALLBACK_PRE(logical, context.instance_location());
result = true;
Expand All @@ -751,16 +747,13 @@ auto evaluate_step(
}
}

CALLBACK_POST("SchemaCompilerLogicalWhenNoAdjacentAnnotations", logical);
} else if (std::holds_alternative<
SchemaCompilerLogicalWhenAdjacentAnnotations>(step)) {
SOURCEMETA_TRACE_START(trace_id,
"SchemaCompilerLogicalWhenAdjacentAnnotations");
const auto &logical{
std::get<SchemaCompilerLogicalWhenAdjacentAnnotations>(step)};
CALLBACK_POST("SchemaCompilerLogicalWhenUnmarked", logical);
} else if (std::holds_alternative<SchemaCompilerLogicalWhenMarked>(step)) {
SOURCEMETA_TRACE_START(trace_id, "SchemaCompilerLogicalWhenMarked");
const auto &logical{std::get<SchemaCompilerLogicalWhenMarked>(step)};
context.push(logical);
EVALUATE_CONDITION_GUARD("SchemaCompilerLogicalWhenAdjacentAnnotations",
logical, instance);
EVALUATE_CONDITION_GUARD("SchemaCompilerLogicalWhenMarked", logical,
instance);
const auto &value{context.resolve_value(logical.value, instance)};

// TODO: How can we avoid this expensive pointer manipulation?
Expand All @@ -769,9 +762,8 @@ auto evaluate_step(
expected_evaluate_path.push_back({value});
const auto &current_annotations{context.annotations(
context.instance_location(), expected_evaluate_path)};
EVALUATE_IMPLICIT_PRECONDITION(
"SchemaCompilerLogicalWhenAdjacentAnnotations", logical,
!current_annotations.empty());
EVALUATE_IMPLICIT_PRECONDITION("SchemaCompilerLogicalWhenMarked", logical,
!current_annotations.empty());

CALLBACK_PRE(logical, context.instance_location());
result = true;
Expand All @@ -782,7 +774,7 @@ auto evaluate_step(
}
}

CALLBACK_POST("SchemaCompilerLogicalWhenAdjacentAnnotations", logical);
CALLBACK_POST("SchemaCompilerLogicalWhenMarked", logical);
} else if (std::holds_alternative<SchemaCompilerLogicalXor>(step)) {
SOURCEMETA_TRACE_START(trace_id, "SchemaCompilerLogicalXor");
const auto &logical{std::get<SchemaCompilerLogicalXor>(step)};
Expand Down Expand Up @@ -826,21 +818,28 @@ auto evaluate_step(
}

CALLBACK_POST("SchemaCompilerLogicalXor", logical);
} else if (std::holds_alternative<SchemaCompilerLogicalTry>(step)) {
SOURCEMETA_TRACE_START(trace_id, "SchemaCompilerLogicalTry");
const auto &logical{std::get<SchemaCompilerLogicalTry>(step)};
} else if (std::holds_alternative<SchemaCompilerLogicalTryMark>(step)) {
SOURCEMETA_TRACE_START(trace_id, "SchemaCompilerLogicalTryMark");
const auto &logical{std::get<SchemaCompilerLogicalTryMark>(step)};
assert(std::holds_alternative<SchemaCompilerValueNone>(logical.value));
context.push(logical);
EVALUATE_CONDITION_GUARD("SchemaCompilerLogicalTry", logical, instance);
EVALUATE_CONDITION_GUARD("SchemaCompilerLogicalTryMark", logical, instance);
CALLBACK_PRE(logical, context.instance_location());
result = true;
for (const auto &child : logical.children) {
if (!evaluate_step(child, instance, mode, callback, context)) {
result = false;
break;
}
}

CALLBACK_POST("SchemaCompilerLogicalTry", logical);
if (result) {
context.annotate(context.instance_location(), JSON{true});
} else {
result = true;
}

CALLBACK_POST("SchemaCompilerLogicalTryMark", logical);
} else if (std::holds_alternative<SchemaCompilerLogicalNot>(step)) {
SOURCEMETA_TRACE_START(trace_id, "SchemaCompilerLogicalNot");
const auto &logical{std::get<SchemaCompilerLogicalNot>(step)};
Expand Down
8 changes: 3 additions & 5 deletions src/jsonschema/compile_json.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,14 +254,12 @@ struct StepVisitor {
HANDLE_STEP("logical", "or", SchemaCompilerLogicalOr)
HANDLE_STEP("logical", "and", SchemaCompilerLogicalAnd)
HANDLE_STEP("logical", "xor", SchemaCompilerLogicalXor)
HANDLE_STEP("logical", "try", SchemaCompilerLogicalTry)
HANDLE_STEP("logical", "try-mark", SchemaCompilerLogicalTryMark)
HANDLE_STEP("logical", "not", SchemaCompilerLogicalNot)
HANDLE_STEP("logical", "when-type", SchemaCompilerLogicalWhenType)
HANDLE_STEP("logical", "when-defines", SchemaCompilerLogicalWhenDefines)
HANDLE_STEP("logical", "when-no-adjacent-annotations",
SchemaCompilerLogicalWhenNoAdjacentAnnotations)
HANDLE_STEP("logical", "when-adjacent-annotations",
SchemaCompilerLogicalWhenAdjacentAnnotations)
HANDLE_STEP("logical", "when-unmarked", SchemaCompilerLogicalWhenUnmarked)
HANDLE_STEP("logical", "when-marked", SchemaCompilerLogicalWhenMarked)
HANDLE_STEP("loop", "properties-match", SchemaCompilerLoopPropertiesMatch)
HANDLE_STEP("loop", "properties", SchemaCompilerLoopProperties)
HANDLE_STEP("loop", "properties-regex", SchemaCompilerLoopPropertiesRegex)
Expand Down
20 changes: 6 additions & 14 deletions src/jsonschema/default_compiler_draft7.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,11 @@ auto compiler_draft7_applicator_if(
const SchemaCompilerSchemaContext &schema_context,
const SchemaCompilerDynamicContext &dynamic_context)
-> SchemaCompilerTemplate {
SchemaCompilerTemplate children{compile(context, schema_context,
relative_dynamic_context,
empty_pointer, empty_pointer)};
children.push_back(make<SchemaCompilerAnnotationEmit>(
true, context, schema_context, relative_dynamic_context, JSON{true}, {},
SchemaCompilerTargetType::Instance));
// TODO: Make this a "try-and-mark" step that internally emits the annotation
// Maybe then we make the other rules about "marked", "unmarked", without
// pointing out the fact that annotations are produced? It would also
// avoid emitting this unnecessary annotation
return {make<SchemaCompilerLogicalTry>(
return {make<SchemaCompilerLogicalTryMark>(
true, context, schema_context, dynamic_context, SchemaCompilerValueNone{},
std::move(children), SchemaCompilerTemplate{})};
compile(context, schema_context, relative_dynamic_context, empty_pointer,
empty_pointer),
SchemaCompilerTemplate{})};
}

auto compiler_draft7_applicator_then(
Expand All @@ -41,7 +33,7 @@ auto compiler_draft7_applicator_then(
return {};
}

return {make<SchemaCompilerLogicalWhenAdjacentAnnotations>(
return {make<SchemaCompilerLogicalWhenMarked>(
true, context, schema_context, dynamic_context, "if",
compile(context, schema_context, relative_dynamic_context, empty_pointer,
empty_pointer),
Expand All @@ -60,7 +52,7 @@ auto compiler_draft7_applicator_else(
return {};
}

return {make<SchemaCompilerLogicalWhenNoAdjacentAnnotations>(
return {make<SchemaCompilerLogicalWhenUnmarked>(
true, context, schema_context, dynamic_context, "if",
compile(context, schema_context, relative_dynamic_context, empty_pointer,
empty_pointer),
Expand Down
25 changes: 11 additions & 14 deletions src/jsonschema/include/sourcemeta/jsontoolkit/jsonschema_compile.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,8 @@ struct SchemaCompilerLogicalXor;

/// @ingroup jsonschema
/// Represents a compiler logical step that represents a conjunction that always
/// reports success
struct SchemaCompilerLogicalTry;
/// reports success and marks its outcome for other steps
struct SchemaCompilerLogicalTryMark;

/// @ingroup jsonschema
/// Represents a compiler logical step that represents a negation
Expand All @@ -261,13 +261,13 @@ struct SchemaCompilerLogicalWhenDefines;

/// @ingroup jsonschema
/// Represents a compiler logical step that represents a conjunction
/// when the instance did not receive any adjacent annotation
struct SchemaCompilerLogicalWhenNoAdjacentAnnotations;
/// when the instance and desired evaluation path was not marked
struct SchemaCompilerLogicalWhenUnmarked;

/// @ingroup jsonschema
/// Represents a compiler logical step that represents a conjunction
/// when the instance received any adjacent annotation
struct SchemaCompilerLogicalWhenAdjacentAnnotations;
/// when the instance and desired evaluation path was marked
struct SchemaCompilerLogicalWhenMarked;

/// @ingroup jsonschema
/// Represents a compiler step that matches steps to object properties
Expand Down Expand Up @@ -340,10 +340,9 @@ using SchemaCompilerTemplate = std::vector<std::variant<
SchemaCompilerAssertionNoAnnotation, SchemaCompilerAnnotationEmit,
SchemaCompilerAnnotationToParent, SchemaCompilerAnnotationBasenameToParent,
SchemaCompilerLogicalOr, SchemaCompilerLogicalAnd, SchemaCompilerLogicalXor,
SchemaCompilerLogicalTry, SchemaCompilerLogicalNot,
SchemaCompilerLogicalTryMark, SchemaCompilerLogicalNot,
SchemaCompilerLogicalWhenType, SchemaCompilerLogicalWhenDefines,
SchemaCompilerLogicalWhenNoAdjacentAnnotations,
SchemaCompilerLogicalWhenAdjacentAnnotations,
SchemaCompilerLogicalWhenUnmarked, SchemaCompilerLogicalWhenMarked,
SchemaCompilerLoopPropertiesMatch, SchemaCompilerLoopProperties,
SchemaCompilerLoopPropertiesRegex,
SchemaCompilerLoopPropertiesNoAdjacentAnnotation, SchemaCompilerLoopKeys,
Expand Down Expand Up @@ -436,14 +435,12 @@ DEFINE_STEP_WITH_VALUE(Annotation, BasenameToParent, SchemaCompilerValueNone)
DEFINE_STEP_APPLICATOR(Logical, Or, SchemaCompilerValueBoolean)
DEFINE_STEP_APPLICATOR(Logical, And, SchemaCompilerValueNone)
DEFINE_STEP_APPLICATOR(Logical, Xor, SchemaCompilerValueNone)
DEFINE_STEP_APPLICATOR(Logical, Try, SchemaCompilerValueNone)
DEFINE_STEP_APPLICATOR(Logical, TryMark, SchemaCompilerValueNone)
DEFINE_STEP_APPLICATOR(Logical, Not, SchemaCompilerValueNone)
DEFINE_STEP_APPLICATOR(Logical, WhenType, SchemaCompilerValueType)
DEFINE_STEP_APPLICATOR(Logical, WhenDefines, SchemaCompilerValueString)
DEFINE_STEP_APPLICATOR(Logical, WhenNoAdjacentAnnotations,
SchemaCompilerValueString)
DEFINE_STEP_APPLICATOR(Logical, WhenAdjacentAnnotations,
SchemaCompilerValueString)
DEFINE_STEP_APPLICATOR(Logical, WhenUnmarked, SchemaCompilerValueString)
DEFINE_STEP_APPLICATOR(Logical, WhenMarked, SchemaCompilerValueString)
DEFINE_STEP_APPLICATOR(Loop, PropertiesMatch, SchemaCompilerValueNamedIndexes)
DEFINE_STEP_APPLICATOR(Loop, Properties, SchemaCompilerValueNone)
DEFINE_STEP_APPLICATOR(Loop, PropertiesRegex, SchemaCompilerValueRegex)
Expand Down
Loading

4 comments on commit 5165ffb

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark (macos)

Benchmark suite Current: 5165ffb Previous: adf7305 Ratio
JSONSchema_Compile_Basic 216568.0071695568 ns/iter 197746.89795918885 ns/iter 1.10
JSONSchema_Validate_Draft4_Meta_1_No_Callback 6610.233369352394 ns/iter 6014.0444764863305 ns/iter 1.10
JSONSchema_Validate_Draft4_Required_Properties 3646.1036065253793 ns/iter 3416.530299187251 ns/iter 1.07
JSONSchema_Validate_Draft4_Optional_Properties_Minimal_Match 1295.4398624418723 ns/iter 1198.6364680905672 ns/iter 1.08
JSONSchema_Validate_Draft4_Items_Schema 9804.167022549027 ns/iter 9002.564715278142 ns/iter 1.09

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark (linux/llvm)

Benchmark suite Current: 5165ffb Previous: adf7305 Ratio
JSONSchema_Compile_Basic 388492.1071829152 ns/iter 393242.6071428439 ns/iter 0.99
JSONSchema_Validate_Draft4_Meta_1_No_Callback 6132.665899922391 ns/iter 5985.251449430998 ns/iter 1.02
JSONSchema_Validate_Draft4_Required_Properties 3342.569075828852 ns/iter 3332.339888755645 ns/iter 1.00
JSONSchema_Validate_Draft4_Optional_Properties_Minimal_Match 811.8733566622279 ns/iter 797.3393270655973 ns/iter 1.02
JSONSchema_Validate_Draft4_Items_Schema 10330.8688879653 ns/iter 10351.026556681616 ns/iter 1.00

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark (linux/gcc)

Benchmark suite Current: 5165ffb Previous: adf7305 Ratio
JSONSchema_Validate_Draft4_Meta_1_No_Callback 6272.835288828571 ns/iter 6249.846650315542 ns/iter 1.00
JSONSchema_Validate_Draft4_Required_Properties 3495.958208820784 ns/iter 3513.4596351121663 ns/iter 1.00
JSONSchema_Validate_Draft4_Optional_Properties_Minimal_Match 831.8309535988003 ns/iter 824.4736643282225 ns/iter 1.01
JSONSchema_Validate_Draft4_Items_Schema 12333.739910236303 ns/iter 12026.330718667257 ns/iter 1.03
JSONSchema_Compile_Basic 387044.3375760925 ns/iter 390307.01387345174 ns/iter 0.99

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark (windows)

Benchmark suite Current: 5165ffb Previous: adf7305 Ratio
JSONSchema_Compile_Basic 844704.5758927121 ns/iter 793118.8616071085 ns/iter 1.07
JSONSchema_Validate_Draft4_Meta_1_No_Callback 13159.48210052974 ns/iter 12955.476785711946 ns/iter 1.02
JSONSchema_Validate_Draft4_Required_Properties 5502.762999999504 ns/iter 5359.2214285710525 ns/iter 1.03
JSONSchema_Validate_Draft4_Optional_Properties_Minimal_Match 2289.602187499895 ns/iter 2325.4324999996356 ns/iter 0.98
JSONSchema_Validate_Draft4_Items_Schema 27442.621238304007 ns/iter 25842.693431492753 ns/iter 1.06

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.