Skip to content

Commit 9e3c1e0

Browse files
iahsfacebook-github-bot
authored andcommitted
Add incremental resolver for SyntaxGraph
Summary: Enables building up SyntaxGraph as subgraphs are accessed instead of paying the full cost upfront. Internally synchronizes accesses to the data structures to provide thread safety. Reviewed By: praihan Differential Revision: D72751923 fbshipit-source-id: 5d6fda62969f3fc5a79cf65f1982efa91bf72772
1 parent cc244c2 commit 9e3c1e0

File tree

6 files changed

+170
-6
lines changed

6 files changed

+170
-6
lines changed

third-party/thrift/src/thrift/lib/cpp2/runtime/SchemaRegistry.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@
2424

2525
namespace apache::thrift {
2626

27+
SchemaRegistry::SchemaRegistry(BaseSchemaRegistry& base) : base_(base) {
28+
auto resolver = std::make_unique<schema::detail::IncrementalResolver>();
29+
resolver_ = resolver.get();
30+
syntaxGraph_ = std::make_unique<schema::SyntaxGraph>(std::move(resolver));
31+
}
32+
SchemaRegistry::~SchemaRegistry() = default;
33+
2734
SchemaRegistry& SchemaRegistry::get() {
2835
static folly::Indestructible<SchemaRegistry> self(BaseSchemaRegistry::get());
2936
return *self;

third-party/thrift/src/thrift/lib/cpp2/runtime/SchemaRegistry.h

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@
2323
#include <unordered_set>
2424
#include <folly/synchronization/RelaxedAtomic.h>
2525
#include <thrift/lib/cpp2/runtime/BaseSchemaRegistry.h>
26+
#include <thrift/lib/cpp2/schema/SyntaxGraph.h>
27+
#include <thrift/lib/cpp2/schema/detail/SchemaBackedResolver.h>
28+
#include <thrift/lib/thrift/gen-cpp2/schema_types.h>
2629

2730
namespace apache::thrift {
2831

@@ -36,13 +39,25 @@ class SchemaRegistry {
3639
// Access all data registered
3740
Ptr getMergedSchema();
3841

39-
explicit SchemaRegistry(BaseSchemaRegistry& base) : base_(base) {}
42+
/**
43+
* Gets node for given definition, or throws `std::out_of_range` if not
44+
* present in schema.
45+
*/
46+
template <typename T>
47+
const schema::DefinitionNode& getDefinitionNode() const {
48+
return resolver_->getDefinitionNode<T>();
49+
}
50+
51+
explicit SchemaRegistry(BaseSchemaRegistry& base);
52+
~SchemaRegistry();
4053

4154
private:
4255
BaseSchemaRegistry& base_;
4356
Ptr mergedSchema_;
4457
folly::relaxed_atomic<bool> mergedSchemaAccessed_{false};
4558
std::unordered_set<type::ProgramId> includedPrograms_;
59+
std::unique_ptr<schema::SyntaxGraph> syntaxGraph_;
60+
schema::detail::IncrementalResolver* resolver_;
4661
};
4762

4863
} // namespace apache::thrift

third-party/thrift/src/thrift/lib/cpp2/schema/SyntaxGraph.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,11 +1449,11 @@ class SyntaxGraph final {
14491449
*/
14501450
ProgramNode::IncludesList programs() const;
14511451

1452+
explicit SyntaxGraph(std::unique_ptr<detail::Resolver> resolver);
1453+
14521454
private:
14531455
folly::not_null_unique_ptr<const detail::Resolver> resolver_;
14541456

1455-
explicit SyntaxGraph(std::unique_ptr<detail::Resolver> resolver);
1456-
14571457
friend const DefinitionNode& detail::lookUpDefinition(
14581458
const SyntaxGraph&, const apache::thrift::type::DefinitionKey&);
14591459
};

third-party/thrift/src/thrift/lib/cpp2/schema/detail/SchemaBackedResolver.cpp

Lines changed: 79 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <thrift/lib/cpp2/schema/detail/SchemaBackedResolver.h>
1818

1919
#include <thrift/lib/cpp2/schema/SyntaxGraph.h>
20+
#include <thrift/lib/cpp2/schema/detail/Merge.h>
2021

2122
#ifdef THRIFT_SCHEMA_AVAILABLE
2223

@@ -69,12 +70,12 @@ class DefinitionKeyEqual : public std::equal_to<type::DefinitionKey> {
6970
return Delegate::operator()(lhs.get(), rhs);
7071
}
7172
};
73+
} // namespace
7274

7375
class SchemaIndex {
7476
public:
7577
explicit SchemaIndex(Resolver& resolver) : resolver_(resolver) {}
7678

77-
private:
7879
// Annotations are referred to in schema.thrift using DefinitionKey, not
7980
// TypeStruct. Therefore, we use this map to keep their resolved TypeStruct
8081
// instances alive.
@@ -784,8 +785,6 @@ const type::Type* SchemaIndex::tryGetAnnotationType(
784785
return std::addressof(element->second);
785786
}
786787

787-
} // namespace
788-
789788
folly::not_null_unique_ptr<Resolver> createResolverfromSchema(
790789
type::Schema&& schema) {
791790
return std::make_unique<FullyResolvedSchemaBackedResolver>(std::move(schema));
@@ -795,5 +794,82 @@ folly::not_null_unique_ptr<Resolver> createResolverfromSchemaRef(
795794
return std::make_unique<FullyResolvedSchemaRefBackedResolver>(schema);
796795
}
797796

797+
IncrementalResolver::IncrementalResolver()
798+
: index_(std::make_unique<SchemaIndex>(*this)) {}
799+
IncrementalResolver::~IncrementalResolver() = default;
800+
801+
const ProgramNode& IncrementalResolver::programOf(
802+
const type::ProgramId& id) const {
803+
return index_->programOf(id);
804+
}
805+
const protocol::Value& IncrementalResolver::valueOf(
806+
const type::ValueId& id) const {
807+
return index_->valueOf(id);
808+
}
809+
const DefinitionNode* IncrementalResolver::definitionOf(
810+
const type::DefinitionKey& key) const {
811+
return index_->definitionOf(key);
812+
}
813+
ProgramNode::IncludesList IncrementalResolver::programs() const {
814+
return index_->programs();
815+
}
816+
817+
const DefinitionNode& IncrementalResolver::getDefinitionNode(
818+
const type::DefinitionKey& key,
819+
type::ProgramId programId,
820+
std::string_view name,
821+
::folly::Range<const ::std::string_view*> (*bundle)()) const {
822+
{
823+
auto schemaReadGuard = schema_.rlock();
824+
if (auto* def = index_->definitionOf(key)) {
825+
return *def;
826+
}
827+
}
828+
829+
if (!bundle) {
830+
folly::throw_exception<std::out_of_range>(
831+
fmt::format("Definition `{}` does not have bundled schema.", name));
832+
}
833+
834+
auto schemaWriteGuard = schema_.wlock();
835+
if (auto* def = index_->definitionOf(key)) {
836+
return *def;
837+
}
838+
839+
auto src = mergeSchemas((*bundle)());
840+
auto& dst = *schemaWriteGuard;
841+
842+
// Merge new schema data in
843+
// TODO: avoid deserializing shared deps
844+
std::copy_if(
845+
std::make_move_iterator(src.programs()->begin()),
846+
std::make_move_iterator(src.programs()->end()),
847+
std::back_inserter(*dst.programs()),
848+
[this](const auto& program) {
849+
return index_->programsById_.count(*program.id()) == 0;
850+
});
851+
dst.valuesMap()->insert(
852+
std::make_move_iterator(src.valuesMap()->begin()),
853+
std::make_move_iterator(src.valuesMap()->end()));
854+
dst.definitionsMap()->insert(
855+
std::make_move_iterator(src.definitionsMap()->begin()),
856+
std::make_move_iterator(src.definitionsMap()->end()));
857+
858+
// Rebuild indices
859+
// TODO: incremental rebuild
860+
index_->init(dst);
861+
862+
if (auto* def = index_->definitionOf(key)) {
863+
return *def;
864+
}
865+
866+
if (index_->programsById_.count(programId)) {
867+
folly::throw_exception<InvalidSyntaxGraphError>(fmt::format(
868+
"Definition `{}` not found in its program's schema.", name));
869+
}
870+
folly::throw_exception<std::out_of_range>(
871+
fmt::format("Definition `{}` does not have bundled schema.", name));
872+
}
873+
798874
} // namespace apache::thrift::schema::detail
799875
#endif

third-party/thrift/src/thrift/lib/cpp2/schema/detail/SchemaBackedResolver.h

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,56 @@
1919
#include <thrift/lib/cpp2/schema/SchemaV1.h>
2020
#include <thrift/lib/cpp2/schema/detail/Resolver.h>
2121

22+
#include <folly/Synchronized.h>
23+
2224
#ifdef THRIFT_SCHEMA_AVAILABLE
2325

2426
namespace apache::thrift::schema::detail {
2527
namespace type = apache::thrift::type;
28+
class SchemaIndex;
2629

2730
folly::not_null_unique_ptr<Resolver> createResolverfromSchema(type::Schema&&);
2831
folly::not_null_unique_ptr<Resolver> createResolverfromSchemaRef(
2932
const type::Schema&);
3033

34+
class IncrementalResolver : public Resolver {
35+
public:
36+
IncrementalResolver();
37+
~IncrementalResolver() override;
38+
39+
/**
40+
* Gets node for given definition, or throws `std::out_of_range` if not
41+
* present in schema.
42+
*/
43+
template <typename T>
44+
const DefinitionNode& getDefinitionNode() const;
45+
46+
const ProgramNode& programOf(const type::ProgramId& id) const override;
47+
const protocol::Value& valueOf(const type::ValueId& id) const override;
48+
const DefinitionNode* definitionOf(
49+
const type::DefinitionKey& key) const override;
50+
std::vector<folly::not_null<const ProgramNode*>> programs() const override;
51+
52+
private:
53+
const DefinitionNode& getDefinitionNode(
54+
const type::DefinitionKey& key,
55+
type::ProgramId programId,
56+
std::string_view name,
57+
::folly::Range<const ::std::string_view*> (*bundle)()) const;
58+
59+
mutable folly::Synchronized<type::Schema> schema_;
60+
folly::not_null_unique_ptr<SchemaIndex> index_;
61+
};
62+
63+
template <typename T>
64+
const DefinitionNode& IncrementalResolver::getDefinitionNode() const {
65+
using ::apache::thrift::detail::TSchemaAssociation;
66+
return getDefinitionNode(
67+
type::DefinitionKey{TSchemaAssociation<T>::definitionKey},
68+
type::ProgramId{TSchemaAssociation<T>::programId},
69+
folly::pretty_name<T>(),
70+
TSchemaAssociation<T>::bundle);
71+
}
72+
3173
} // namespace apache::thrift::schema::detail
3274
#endif

third-party/thrift/src/thrift/lib/cpp2/schema/test/SyntaxGraphTest.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919

2020
#include <folly/Utility.h>
2121

22+
#include <thrift/lib/cpp2/runtime/SchemaRegistry.h>
2223
#include <thrift/lib/cpp2/schema/SyntaxGraph.h>
2324

25+
#include <thrift/annotation/gen-cpp2/thrift_types.h>
2426
#include <thrift/lib/cpp2/schema/test/gen-cpp2/syntax_graph_2_handlers.h>
2527
#include <thrift/lib/cpp2/schema/test/gen-cpp2/syntax_graph_handlers.h>
2628
#include <thrift/lib/cpp2/schema/test/gen-cpp2/syntax_graph_types.h>
@@ -461,4 +463,26 @@ TEST_F(ServiceSchemaTest, LookupByDefinitionKeys) {
461463
}
462464
}
463465

466+
TEST(SyntaxGraphTest, getDefinitionNode) {
467+
auto& registry = SchemaRegistry::get();
468+
469+
const DefinitionNode& testStruct =
470+
registry.getDefinitionNode<test::TestStruct>();
471+
const StructNode& stct = testStruct.asStruct();
472+
EXPECT_EQ(stct.definition().name(), "TestStruct");
473+
474+
const DefinitionNode& testEnum = registry.getDefinitionNode<test::TestEnum>();
475+
const EnumNode& enm = testEnum.asEnum();
476+
EXPECT_EQ(enm.definition().name(), "TestEnum");
477+
478+
const DefinitionNode& testService =
479+
registry.getDefinitionNode<test::TestService>();
480+
const ServiceNode& serv = testService.asService();
481+
EXPECT_EQ(serv.definition().name(), "TestService");
482+
483+
EXPECT_THROW(
484+
registry.getDefinitionNode<facebook::thrift::annotation::Experimental>(),
485+
std::out_of_range);
486+
}
487+
464488
} // namespace apache::thrift::schema

0 commit comments

Comments
 (0)