[GIE/Runtime] Support Entry Trait in GIE/Runtime
#2300
Conversation
…mon used type. Fix CI Tests.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2300 +/- ##
=======================================
Coverage 39.95% 39.95%
=======================================
Files 88 88
Lines 9757 9757
=======================================
Hits 3898 3898
Misses 5859 5859 Continue to review full report at Codecov.
|
|
|
||
| #[derive(Debug)] | ||
| pub enum EntryDataType { | ||
| Vid, // A vertex global id |
There was a problem hiding this comment.
Do not assume Vid. Id is fine.
There was a problem hiding this comment.
Use Vid here since we may need to specify whether it is a Vid or Eid when getting properties in Auxilia. It would be modified as an Id once we separate GetVProp and GetEProp from Auxilia.
| fn eq(&self, other: &Self) -> bool { | ||
| match (self, other) { | ||
| (EntryDataType::Vid, EntryDataType::Vid) | ||
| | (EntryDataType::Vid, EntryDataType::V) |
There was a problem hiding this comment.
Why V and Vid should be compared to equal?
There was a problem hiding this comment.
Vid (for now) denotes an IdOnlyVertex, and can be compared with a V. Later, it can also be reffered as an IdOnlyEdge, etc.
|
|
||
| pub trait Entry: Debug + Send + Sync + AsAny + Element { | ||
| fn get_type(&self) -> EntryDataType; | ||
| fn as_vid(&self) -> Option<ID> { |
| impl Eq for DynEntry {} | ||
|
|
||
| // demanded when need to group (ToSum) the entry; | ||
| impl std::ops::Add for DynEntry { |
There was a problem hiding this comment.
Feels like we should not implement Add for entry.
There was a problem hiding this comment.
Done. Move this logic into the implementation of Accumulator.
| P, | ||
| Obj, | ||
| Intersect, | ||
| Collection, |
There was a problem hiding this comment.
Please comment these entry options.
| } | ||
|
|
||
| #[derive(Debug, Clone, Default)] | ||
| pub struct IDEntry { |
There was a problem hiding this comment.
IdEntry. BTW, for the next a few definitions, can put VertexEntry, EdgeEntry, PathEntry, IntersectionEntry and CollectionEntry. Anyway, if we already have DynEntry, why we should have so many different entry definitions?
BingqingLyu
force-pushed
the
runtime_entry
branch
from
29b2674
to
dbf83dc
Compare
| } | ||
| } | ||
|
|
||
| impl From<Vertex> for DynEntry { |
There was a problem hiding this comment.
Why not implement
impl<E: Entry + 'static> From<E> for DynEntry { ... }
Then the new() function in DynEntry can be removed.
There was a problem hiding this comment.
Cannot do this since DynEntry is also an Entry, and we'll get the error:
error[E0119]: conflicting implementations of trait `std::convert::From<process::entry::DynEntry>` for type `process::entry::DynEntry`
--> runtime/src/process/entry.rs:574:1
|
574 | impl<E: Entry + 'static> From<E> for DynEntry {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: conflicting implementation in crate `core`:
- impl<T> From<T> for T;
f4761fb
to
a315854
Compare
| pub struct Intersection { | ||
| vertex_vec: Vec<Vertex>, | ||
| pub struct IntersectionEntry { | ||
| vertex_vec: Vec<ID>, |
There was a problem hiding this comment.
Also comment in details for what the IntersectionEntry is, and how it will be used.
What do these changes do?
As titled.
A test of PatternMatch on LDBC.30 shows a 10-40% acceleration in time cost (when using a
IdEntryinExpandItersect).Related issue number
Fixes #2317