Skip to content

Commit 6e06923

Browse files
jviottijviotti
authored
Better encapsulate required checks in properties optimisations (#778)
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
1 parent a0f67b7 commit 6e06923

2 files changed

Lines changed: 56 additions & 78 deletions

File tree

‎src/compiler/compile_helpers.h‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,35 @@ is_circular(const sourcemeta::core::SchemaFrame &frame,
355355
return false;
356356
}
357357

358+
// The set of property names that this schema declares as required at this
359+
// level
360+
inline auto required_properties(const SchemaContext &schema_context)
361+
-> ValueStringSet {
362+
using Known = sourcemeta::core::Vocabularies::Known;
363+
const auto imports_validation_vocabulary{
364+
schema_context.vocabularies.contains(Known::JSON_Schema_Draft_4) ||
365+
schema_context.vocabularies.contains(Known::JSON_Schema_Draft_6) ||
366+
schema_context.vocabularies.contains(Known::JSON_Schema_Draft_7) ||
367+
schema_context.vocabularies.contains(
368+
Known::JSON_Schema_2019_09_Validation) ||
369+
schema_context.vocabularies.contains(
370+
Known::JSON_Schema_2020_12_Validation)};
371+
372+
ValueStringSet result;
373+
374+
if (imports_validation_vocabulary && schema_context.schema.is_object() &&
375+
schema_context.schema.defines("required") &&
376+
schema_context.schema.at("required").is_array()) {
377+
for (const auto &entry : schema_context.schema.at("required").as_array()) {
378+
if (entry.is_string()) {
379+
result.insert(entry.to_string());
380+
}
381+
}
382+
}
383+
384+
return result;
385+
}
386+
358387
} // namespace sourcemeta::blaze
359388

360389
#endif

‎src/compiler/default_compiler_draft3.h‎

Lines changed: 27 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -232,17 +232,11 @@ auto properties_as_loop(const Context &context,
232232
Known::JSON_Schema_2019_09_Validation) ||
233233
schema_context.vocabularies.contains(
234234
Known::JSON_Schema_2020_12_Validation);
235-
std::set<std::string> required;
236-
if (imports_validation_vocabulary &&
237-
schema_context.schema.defines("required") &&
238-
schema_context.schema.at("required").is_array()) {
239-
for (const auto &property :
240-
schema_context.schema.at("required").as_array()) {
241-
if (property.is_string() &&
242-
// Only count the required property if its indeed in "properties"
243-
properties.defines(property.to_string())) {
244-
required.insert(property.to_string());
245-
}
235+
ValueStringSet required;
236+
for (const auto &entry : required_properties(schema_context)) {
237+
// Only count the required property if its indeed in "properties"
238+
if (properties.defines(entry.first)) {
239+
required.insert(entry.first);
246240
}
247241
}
248242

@@ -460,30 +454,19 @@ auto compiler_draft3_applicator_properties_with_options(
460454
schema_context.schema.at("type").to_string() ==
461455
"object"};
462456

463-
using Known = sourcemeta::core::Vocabularies::Known;
464-
const auto imports_top_level_required{
465-
schema_context.vocabularies.contains(Known::JSON_Schema_Draft_4) ||
466-
schema_context.vocabularies.contains(Known::JSON_Schema_Draft_6) ||
467-
schema_context.vocabularies.contains(Known::JSON_Schema_Draft_7) ||
468-
schema_context.vocabularies.contains(
469-
Known::JSON_Schema_2019_09_Validation) ||
470-
schema_context.vocabularies.contains(
471-
Known::JSON_Schema_2020_12_Validation)};
457+
ValueStringSet required{required_properties(schema_context)};
472458

473459
auto properties{compile_properties(context, schema_context,
474460
effective_dynamic_context, current)};
475461

476-
if (context.mode == Mode::FastValidation && imports_top_level_required &&
462+
if (context.mode == Mode::FastValidation && !required.empty() &&
477463
schema_context.schema.defines("additionalProperties") &&
478464
schema_context.schema.at("additionalProperties").is_boolean() &&
479465
!schema_context.schema.at("additionalProperties").to_boolean() &&
480-
schema_context.schema.defines("required") &&
481-
schema_context.schema.at("required").is_array() &&
482-
schema_context.schema.at("required").size() ==
466+
required.size() ==
483467
schema_context.schema.at(dynamic_context.keyword).size() &&
484-
std::ranges::all_of(properties, [&schema_context](const auto &property) {
485-
return schema_context.schema.at("required")
486-
.contains(sourcemeta::core::JSON{property.first});
468+
std::ranges::all_of(properties, [&required](const auto &property) {
469+
return required.contains(property.first);
487470
})) {
488471
if (std::ranges::all_of(properties, [](const auto &property) {
489472
return property.second.size() == 1 &&
@@ -497,11 +480,7 @@ auto compiler_draft3_applicator_properties_with_options(
497480

498481
if (types.size() == 1 &&
499482
!schema_context.schema.defines("patternProperties")) {
500-
if (imports_top_level_required &&
501-
schema_context.schema.defines("required") && assume_object) {
502-
auto required_copy = schema_context.schema.at("required");
503-
std::ranges::sort(required_copy.as_array());
504-
ValueStringSet required{json_array_to_string_set(required_copy)};
483+
if (assume_object) {
505484
if (is_closed_properties_required(schema_context.schema, required)) {
506485
sourcemeta::core::PropertyHashJSON<ValueString> hasher;
507486
std::vector<std::pair<ValueString, ValueStringSet::hash_type>>
@@ -744,12 +723,7 @@ auto compiler_draft3_applicator_properties_with_options(
744723
if (fusion_possible && substeps.size() == 1 &&
745724
substeps.front().type != InstructionIndex::ControlJump &&
746725
substeps.front().type != InstructionIndex::ControlDynamicAnchorJump) {
747-
const auto is_required{
748-
assume_object && imports_top_level_required &&
749-
schema_context.schema.defines("required") &&
750-
schema_context.schema.at("required").is_array() &&
751-
schema_context.schema.at("required")
752-
.contains(sourcemeta::core::JSON{name})};
726+
const auto is_required{assume_object && required.contains(name)};
753727
auto prop{make_property(name)};
754728
auto fusion_child{substeps.front()};
755729
fusion_child.relative_instance_location = {};
@@ -767,11 +741,7 @@ auto compiler_draft3_applicator_properties_with_options(
767741
if (!substeps.empty()) {
768742
// As a performance shortcut
769743
if (effective_dynamic_context.base_instance_location.empty()) {
770-
if (assume_object && imports_top_level_required &&
771-
schema_context.schema.defines("required") &&
772-
schema_context.schema.at("required").is_array() &&
773-
schema_context.schema.at("required")
774-
.contains(sourcemeta::core::JSON{name})) {
744+
if (assume_object && required.contains(name)) {
775745
for (auto &&step : substeps) {
776746
children.push_back(std::move(step));
777747
}
@@ -794,27 +764,19 @@ auto compiler_draft3_applicator_properties_with_options(
794764

795765
if (context.mode == Mode::FastValidation) {
796766
if (fusion_possible && !fusion_entries.empty()) {
797-
if (imports_top_level_required &&
798-
schema_context.schema.defines("required") &&
799-
schema_context.schema.at("required").is_array()) {
800-
for (const auto &req :
801-
schema_context.schema.at("required").as_array()) {
802-
if (!req.is_string()) {
803-
continue;
804-
}
805-
const auto &req_name{req.to_string()};
806-
bool already_tracked{false};
807-
for (const auto &entry : fusion_entries) {
808-
if (std::get<0>(entry) == req_name) {
809-
already_tracked = true;
810-
break;
811-
}
812-
}
813-
if (!already_tracked) {
814-
auto prop{make_property(req_name)};
815-
fusion_entries.emplace_back(prop.first, prop.second, true);
767+
for (const auto &req : required) {
768+
const auto &req_name{req.first};
769+
bool already_tracked{false};
770+
for (const auto &entry : fusion_entries) {
771+
if (std::get<0>(entry) == req_name) {
772+
already_tracked = true;
773+
break;
816774
}
817775
}
776+
if (!already_tracked) {
777+
auto prop{make_property(req_name)};
778+
fusion_entries.emplace_back(prop.first, prop.second, true);
779+
}
818780
}
819781

820782
if (fusion_entries.size() > 32) {
@@ -949,16 +911,6 @@ auto compiler_draft3_applicator_additionalproperties_with_options(
949911
return {};
950912
}
951913

952-
using Known = sourcemeta::core::Vocabularies::Known;
953-
const auto imports_top_level_required{
954-
schema_context.vocabularies.contains(Known::JSON_Schema_Draft_4) ||
955-
schema_context.vocabularies.contains(Known::JSON_Schema_Draft_6) ||
956-
schema_context.vocabularies.contains(Known::JSON_Schema_Draft_7) ||
957-
schema_context.vocabularies.contains(
958-
Known::JSON_Schema_2019_09_Validation) ||
959-
schema_context.vocabularies.contains(
960-
Known::JSON_Schema_2020_12_Validation)};
961-
962914
Instructions children{compile(context, schema_context,
963915
relative_dynamic_context(),
964916
sourcemeta::core::empty_weak_pointer,
@@ -1028,12 +980,9 @@ auto compiler_draft3_applicator_additionalproperties_with_options(
1028980
if (context.mode == Mode::FastValidation && children.size() == 1 &&
1029981
children.front().type == InstructionIndex::AssertionFail &&
1030982
!filter_strings.empty() && filter_prefixes.empty() &&
1031-
filter_regexes.empty() && imports_top_level_required &&
1032-
schema_context.schema.defines("required") &&
1033-
schema_context.schema.at("required").is_array() &&
1034-
is_closed_properties_required(
1035-
schema_context.schema,
1036-
json_array_to_string_set(schema_context.schema.at("required")))) {
983+
filter_regexes.empty() &&
984+
is_closed_properties_required(schema_context.schema,
985+
required_properties(schema_context))) {
1037986
return {};
1038987
}
1039988

0 commit comments

Comments
 (0)