Translation engine catalog optimization #1012
Conversation
…d remove duplication
… ADT infrastructure
…talogOptimization # Conflicts: # third_party/production/TranslationEngineLLVM/clang/lib/Sema/SemaGaia.cpp
…n order of elements in internal data structures
| @@ -610,19 +505,10 @@ bool table_navigation_t::generate_navigation_step(const string& source_table, co | |||
| return true; | |||
| } | |||
|
|
|||
| void table_navigation_t::ensure_initialization() | |||
| llvm::SmallVector<string, 8> table_navigation_t::get_table_fields(const string& table) | |||
There was a problem hiding this comment.
Maybe we can go a little higher than 8 to avoid heap allocation. You can easily expect a table to have more than 8 columns. (maybe 16?)
| @@ -458,6 +459,44 @@ namespace llvm { | |||
| } | |||
| }; | |||
|
|
|||
| template <> | |||
There was a problem hiding this comment.
Can you add documentation about what this is and why did you need to add it (what is the Gaia use-case that we are trying to solve by adding this class).
| void ASTContext::InitBuiltinType(CanQualType &R, BuiltinType::Kind K) { | ||
| auto *Ty = new (*this, TypeAlignment) BuiltinType(K); | ||
| R = CanQualType::CreateUnsafe(QualType(Ty, 0)); | ||
| cacheEDCType(Ty); |
There was a problem hiding this comment.
How do you know this is an EDC type (it doesn't look so to me). Same for the following instances.
There was a problem hiding this comment.
Also, I think we should find a better way than putting cacheEDCType(...); in so many methods within this calss. Maybe we can do it once when the translation engine is initialized?
There was a problem hiding this comment.
Added verification by name.
Not sure if we can do anything else unless we want to make some shortcuts i.e. similar to what Wayne did (his solution doesn't take into account definition context that may impact the correctness of cached types)
| @@ -15,4 +15,4 @@ ruleset test10 | |||
| } | |||
| } | |||
|
|
|||
| // CHECK: Multiple shortest paths exist between tables 'crop' and 'sensor'. Explicitly specify the navigation path between these two tables to resolve the ambiguity. | |||
| // CHECK: Multiple shortest paths exist between tables 'crop' and 'feeding'. Explicitly specify the navigation path between these two tables to resolve the ambiguity. | |||
There was a problem hiding this comment.
Out of curiosity why this changed? Does this mean we had a bug earlier that was fixed by this PR?
There was a problem hiding this comment.
I believe different hashing function led to different order in which tables are iterated and reported. (The schema has multiple tables to which multiple shortest paths exist)
It likely depends on your schema and ruleset. For my change Wayne reported improvement of around 30%. He reported much better improvement for his caching search improvement and since my change has the same caching but implemented slightly differently and hopefully more correctly the improvement should be comparable |
Awesome, thanks! |
…that doesn't have any fields or links
The purpose of this PR is to refactor the gaiat to eliminate duplication of code between llvm, gaiat and gaia, optimize interaction of gaiat with catalog to reduce number of DB transactions, make gaiat and Gaia introduced llvm code to use llvm-native ADT that per some evidence faster then STL ones, various performance optimizations.
There are no functional changes. The users are expected to see noticeable improvement in gaiat performance when used on large rulesets with complex DB schema.