[
https://issues.apache.org/jira/browse/S2GRAPH-122?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15669404#comment-15669404
]
ASF GitHub Bot commented on S2GRAPH-122:
----------------------------------------
Github user SteamShon commented on the issue:
https://github.com/apache/incubator-s2graph/pull/97
Now every test cases passed, and I think it's ready for the reviews. Since
Most used interface on Edge/IndexEdge/SnapshotEdge has been changed, the diff
is giant. Sorry about this for reviewers.
> Change data types of Edge/IndexEdge/SnapshotEdge.
> -------------------------------------------------
>
> Key: S2GRAPH-122
> URL: https://issues.apache.org/jira/browse/S2GRAPH-122
> Project: S2Graph
> Issue Type: Improvement
> Affects Versions: 0.2.0
> Reporter: DOYUNG YOON
> Assignee: DOYUNG YOON
> Priority: Minor
> Labels: performance
> Fix For: 0.2.0
>
> Original Estimate: 96h
> Remaining Estimate: 96h
>
> Currently Edge have following interface.
> {noformat}
> case class Edge(srcVertex: Vertex,
> tgtVertex: Vertex,
> labelWithDir: LabelWithDirection,
> op: Byte = GraphUtil.defaultOpByte,
> version: Long = System.currentTimeMillis(),
> propsWithTs: Map[Byte, InnerValLikeWithTs],
> parentEdges: Seq[EdgeWithScore] = Nil,
> originalEdgeOpt: Option[Edge] = None,
> pendingEdgeOpt: Option[Edge] = None,
> statusCode: Byte = 0,
> lockTs: Option[Long] = None)
> case class IndexEdge(srcVertex: Vertex,
> tgtVertex: Vertex,
> labelWithDir: LabelWithDirection,
> op: Byte,
> version: Long,
> labelIndexSeq: Byte,
> props: Map[Byte, InnerValLikeWithTs])
> case class SnapshotEdge(srcVertex: Vertex,
> tgtVertex: Vertex,
> labelWithDir: LabelWithDirection,
> op: Byte,
> version: Long,
> props: Map[Byte, InnerValLikeWithTs],
> pendingEdgeOpt: Option[Edge],
> statusCode: Byte = 0,
> lockTs: Option[Long])
> {noformat}
> Following is my suggestion.
> 1. I think there is no reason to use `LabelWithDirection` which only have
> labelId and direction. Instead we can actually change it to hold `Label`
> which contains lots of other meta data(ex: write options which decide this
> edge need to store reverse direction or not). Because of using
> `LabelWithDirection`, there are lots of duplicate code to lookup to get
> `Label` instance from `LabelWithDirection`. Even though we are using local
> cache, It would be better if we can remove unnecessary lookup cost. I think
> storing `Label` is also good because we can remove lots of duplicate code
> which find `Label` instance from LabelWithDirection.
> 2. When we deserialize, we first deserialize `SKeyValue` into `IndexEdge`,
> then call `toEdge` to convert `IndexEdge` to `Edge`. when we convert, there
> is unnecessary data copy because `IndexEdge` and `Edge` has different data
> type for props value.
> {noformat}
> props.map { case (k, v) => k -> InnerValLikeWithTs(v, version) }
> {noformat}
> This will be called per each edge that fetched from storage, so we should
> avoid unnecessary iteration of properties.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
