Skip to content

Commit

Permalink
fix(interactive): Remove Unnecessary JNA Structure Creation to Save M…
Browse files Browse the repository at this point in the history
…emory Consumption (#3354)

<!--
Thanks for your contribution! please review
https://github.com/alibaba/GraphScope/blob/main/CONTRIBUTING.md before
opening an issue.
-->

## What do these changes do?
as titled.

<!-- Please give a short brief about these changes. -->

## Related issue number

<!-- Are there any issues opened that will be resolved by merging this
change? -->

Fixes
  • Loading branch information
shirly121 authored Nov 15, 2023
1 parent f20c6a5 commit c7f6b3d
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright 2020 Alibaba Group Holding Limited.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.alibaba.graphscope.common.intermediate;

public class ArgVariable {
private final String tagName;
private final String propertyName;

public ArgVariable(String tagName, String propertyName) {
this.tagName = tagName;
this.propertyName = propertyName;
}

@Override
public String toString() {
return (propertyName == null || propertyName.isEmpty())
? "@" + tagName
: "@" + tagName + "." + propertyName;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package com.alibaba.graphscope.gremlin.transform;

import com.alibaba.graphscope.common.intermediate.ArgUtils;
import com.alibaba.graphscope.common.intermediate.ArgVariable;

import org.apache.tinkerpop.gremlin.process.traversal.P;
import org.apache.tinkerpop.gremlin.process.traversal.Step;
Expand Down Expand Up @@ -64,7 +64,7 @@ public String apply(Step arg) {
public String apply(Step arg) {
WhereTraversalStep.WhereEndStep endStep = (WhereTraversalStep.WhereEndStep) arg;
String matchTag = endStep.getScopeKeys().iterator().next();
P predicate = P.eq(ArgUtils.asVar(matchTag, ""));
P predicate = P.eq(new ArgVariable(matchTag, ""));
return flatPredicate("@", predicate);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -401,17 +401,13 @@ public InterOpBase apply(Step step) {
private String getMapKey(TraversalMapStep step) {
Step groupStep = getPreviousGroupStep(step);
int stepIdx = TraversalHelper.stepIndex(groupStep, groupStep.getTraversal());
FfiAlias.ByValue keyAlias =
AliasManager.getFfiAlias(new AliasArg(AliasPrefixType.GROUP_KEYS, stepIdx));
return keyAlias.alias.name;
return AliasManager.getAliasName(new AliasArg(AliasPrefixType.GROUP_KEYS, stepIdx));
}

private String getMapValue(TraversalMapStep step) {
Step groupStep = getPreviousGroupStep(step);
int stepIdx = TraversalHelper.stepIndex(groupStep, groupStep.getTraversal());
FfiAlias.ByValue valueAlias =
AliasManager.getFfiAlias(new AliasArg(AliasPrefixType.GROUP_VALUES, stepIdx));
return valueAlias.alias.name;
return AliasManager.getAliasName(new AliasArg(AliasPrefixType.GROUP_VALUES, stepIdx));
}

private Step getPreviousGroupStep(Step step) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.alibaba.graphscope.common.exception.OpArgIllegalException;
import com.alibaba.graphscope.common.intermediate.ArgAggFn;
import com.alibaba.graphscope.common.intermediate.ArgUtils;
import com.alibaba.graphscope.common.intermediate.ArgVariable;
import com.alibaba.graphscope.common.intermediate.operator.InterOpBase;
import com.alibaba.graphscope.common.intermediate.operator.ProjectOp;
import com.alibaba.graphscope.common.jna.type.FfiAggOpt;
Expand Down Expand Up @@ -245,6 +246,22 @@ default FfiVariable.ByValue getExpressionAsVar(String expr) {
return ArgUtils.asVar(tag, property);
}

default ArgVariable getExpressionAsArgVar(String expr) {
if (expr.startsWith("{") && expr.endsWith("}")) {
throw new OpArgIllegalException(
OpArgIllegalException.Cause.INVALID_TYPE,
"can not convert expression of valueMap to variable");
}
String[] splitExpr = expr.split("\\.");
if (splitExpr.length == 0) {
throw new OpArgIllegalException(
OpArgIllegalException.Cause.INVALID_TYPE, "expr " + expr + " is invalid");
}
String tag = (splitExpr[0].length() > 0) ? splitExpr[0].substring(1) : splitExpr[0];
String property = (splitExpr.length > 1) ? splitExpr[1] : "";
return new ArgVariable(tag, property);
}

default Map<String, Traversal.Admin> getProjectTraversals(TraversalParent parent) {
if (parent instanceof SelectOneStep) {
SelectOneStep step = (SelectOneStep) parent;
Expand Down Expand Up @@ -272,9 +289,6 @@ default Map<String, Traversal.Admin> getProjectTraversals(TraversalParent parent
// AliasManager in an automatic way or is given by the query)
// and get aggregate variable, set to NONE by default
default ArgAggFn getAggFn(Step step, int stepIdx, int subId) {
FfiAlias.ByValue alias =
AliasManager.getFfiAlias(
new AliasArg(AliasPrefixType.GROUP_VALUES, stepIdx, subId));
FfiAggOpt aggOpt;
if (step instanceof CountGlobalStep) {
aggOpt = FfiAggOpt.Count;
Expand All @@ -293,11 +307,16 @@ default ArgAggFn getAggFn(Step step, int stepIdx, int subId) {
OpArgIllegalException.Cause.UNSUPPORTED_TYPE,
"invalid aggFn " + step.getClass());
}
FfiAlias.ByValue alias;
// group().by(..).by(count().as("a")), "a" is the query given alias of group value
Set<String> labels = step.getLabels();
if (labels != null && !labels.isEmpty()) {
String label = labels.iterator().next();
alias = ArgUtils.asAlias(label, true);
} else {
alias =
AliasManager.getFfiAlias(
new AliasArg(AliasPrefixType.GROUP_VALUES, stepIdx, subId));
}
// set to NONE by default
return new ArgAggFn(aggOpt, alias);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.alibaba.graphscope.common.exception.OpArgIllegalException;
import com.alibaba.graphscope.common.intermediate.ArgAggFn;
import com.alibaba.graphscope.common.intermediate.ArgUtils;
import com.alibaba.graphscope.common.intermediate.ArgVariable;
import com.alibaba.graphscope.common.intermediate.operator.*;
import com.alibaba.graphscope.common.jna.type.*;
import com.alibaba.graphscope.gremlin.InterOpCollectionBuilder;
Expand Down Expand Up @@ -495,7 +496,7 @@ private void traverseAndUpdateP(
stepId,
subId,
applys);
FfiVariable.ByValue var = getExpressionAsVar(expr);
ArgVariable var = getExpressionAsArgVar(expr);
predicate.setValue(var);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ public static FfiAlias.ByValue getFfiAlias(AliasArg prefix) {
return ArgUtils.asAlias(alias, false);
}

public static String getAliasName(AliasArg prefix) {
int stepIdx = prefix.getStepIdx();
int subTraversalId = prefix.getSubTraversalIdx();
return prefix.getPrefix() + "_" + stepIdx + "_" + subTraversalId;
}

// prefix is as the gremlin result key which is used to display
public static String getPrefix(String aliasName) {
String[] splits = aliasName.split("_");
Expand Down

0 comments on commit c7f6b3d

Please sign in to comment.