Skip to content

Commit

Permalink
[MINOR] Avoid unnecessary overhead in createvar instructions
Browse files Browse the repository at this point in the history
This patch makes a minor performance improvement to the createvar
instruction execution (which happens for every non-scalar operator). In
detail, the need for creating unique file names (from one instruction),
led to unnecessary string concatenation and thus object allocation. We
now reuse the existing thread-local string builders as used for
instruction generation.

On a special-case scenario with ~1M loop iterations over tiny data (100
values), this patch improved the createvar overhead from 22.1s to 5.6s
(and overall from 49s to 33s).
  • Loading branch information
mboehm7 committed May 14, 2020
1 parent 996f612 commit 30d5c40
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
Expand Up @@ -1008,4 +1008,12 @@ public static String concatOperands(String... inputs) {
sb.append(inputs[inputs.length-1]);
return sb.toString();
}

public static String concatStrings(String... inputs) {
StringBuilder sb = _strBuilders.get();
sb.setLength(0); //reuse allocated space
for( int i=0; i<inputs.length; i++ )
sb.append(inputs[i]);
return sb.toString();
}
}
Expand Up @@ -521,9 +521,8 @@ public void processInstruction(ExecutionContext ec) {
//(existing objects gets cleared through rmvar instructions)
String fname = getInput2().getName();
// check if unique filename needs to be generated
if( Boolean.parseBoolean(getInput3().getName()) ) {
fname = fname + '_' + _uniqueVarID.getNextID();
}
if( Boolean.parseBoolean(getInput3().getName()) )
fname = getUniqueFileName(fname);
MatrixObject obj = new MatrixObject(getInput1().getValueType(), fname);
//clone meta data because it is updated on copy-on-write, otherwise there
//is potential for hidden side effects between variables.
Expand All @@ -544,9 +543,8 @@ else if( getInput1().getDataType() == DataType.TENSOR ) {
//(existing objects gets cleared through rmvar instructions)
String fname = getInput2().getName();
// check if unique filename needs to be generated
if( Boolean.parseBoolean(getInput3().getName()) ) {
fname = fname + '_' + _uniqueVarID.getNextID();
}
if( Boolean.parseBoolean(getInput3().getName()) )
fname = getUniqueFileName(fname);
CacheableData<?> obj = new TensorObject(getInput1().getValueType(), fname);
//clone meta data because it is updated on copy-on-write, otherwise there
//is potential for hidden side effects between variables.
Expand All @@ -560,6 +558,8 @@ else if( getInput1().getDataType() == DataType.TENSOR ) {
}
else if( getInput1().getDataType() == DataType.FRAME ) {
String fname = getInput2().getName();
if( Boolean.parseBoolean(getInput3().getName()) )
fname = getUniqueFileName(fname);
FrameObject fobj = new FrameObject(fname);
fobj.setMetaData((MetaData)metadata.clone());
fobj.setFileFormatProperties(_formatProperties);
Expand Down Expand Up @@ -1257,4 +1257,8 @@ public boolean isVariableCastInstruction() {
|| opcode == VariableOperationCode.CastAsDoubleVariable
|| opcode == VariableOperationCode.CastAsBooleanVariable;
}

public static String getUniqueFileName(String fname) {
return InstructionUtils.concatStrings(fname, "_", String.valueOf(_uniqueVarID.getNextID()));
}
}

0 comments on commit 30d5c40

Please sign in to comment.