Skip to content

Commit

Permalink
Added ifNot as a more efficient if(not(test))
Browse files Browse the repository at this point in the history
  • Loading branch information
@pontusmelke
pontusmelke committed May 17, 2016
1 parent bbf191e commit 846821a
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 95 deletions.
7 changes: 6 additions & 1 deletion community/codegen/src/main/java/org/neo4j/codegen/CodeBlock.java
View file
Expand Up @@ -24,7 +24,6 @@

import static org.neo4j.codegen.LocalVariables.copy;
import static org.neo4j.codegen.MethodReference.methodReference;
import static org.neo4j.codegen.Resource.withResource;
import static org.neo4j.codegen.TypeReference.typeReference;

public class CodeBlock implements AutoCloseable
Expand Down Expand Up @@ -172,6 +171,12 @@ public CodeBlock ifStatement( Expression test )
return new CodeBlock( this );
}

public CodeBlock ifNotStatement( Expression test )
{
emit( e -> e.beginIfNot( test ) );
return new CodeBlock( this );
}

public void tryCatch( Consumer<CodeBlock> body, Consumer<CodeBlock> onError, Parameter exception )
{
emit( e -> e.tryCatchBlock( body, onError, localVariables.createNew( exception.type(), exception.name() ),
Expand Down
6 changes: 6 additions & 0 deletions community/codegen/src/main/java/org/neo4j/codegen/InvalidState.java
View file
Expand Up @@ -100,6 +100,12 @@ public void beginIf( Expression test )
throw new IllegalStateException( reason );
}

@Override
public void beginIfNot( Expression test )
{
throw new IllegalStateException( reason );
}

@Override
public void endBlock()
{
Expand Down
2 changes: 2 additions & 0 deletions community/codegen/src/main/java/org/neo4j/codegen/MethodEmitter.java
View file
Expand Up @@ -39,6 +39,8 @@ public interface MethodEmitter

void beginIf( Expression test );

void beginIfNot( Expression test );

void endBlock();

<T> void tryCatchBlock( Consumer<T> body, Consumer<T> handler, LocalVariable exception, T block);
Expand Down
23 changes: 14 additions & 9 deletions community/codegen/src/main/java/org/neo4j/codegen/bytecode/MethodByteCodeEmitter.java
View file
Expand Up @@ -24,18 +24,14 @@
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;

import java.util.ArrayList;
import java.util.Deque;
import java.util.LinkedList;
import java.util.List;
import java.util.function.Consumer;

import org.neo4j.codegen.BaseExpressionVisitor;
import org.neo4j.codegen.CatchClause;
import org.neo4j.codegen.Expression;
import org.neo4j.codegen.FieldReference;
import org.neo4j.codegen.LocalVariable;
import org.neo4j.codegen.LocalVariables;
import org.neo4j.codegen.MethodDeclaration;
import org.neo4j.codegen.MethodEmitter;
import org.neo4j.codegen.MethodReference;
Expand All @@ -48,7 +44,6 @@
import static org.neo4j.codegen.ByteCodeUtils.outerName;
import static org.neo4j.codegen.ByteCodeUtils.signature;
import static org.neo4j.codegen.ByteCodeUtils.typeName;
import static org.neo4j.codegen.MethodReference.methodReference;

class MethodByteCodeEmitter implements MethodEmitter, Opcodes
{
Expand Down Expand Up @@ -189,12 +184,16 @@ public void beginIf( Expression test )
callSuperIfNecessary();
test.accept( expressionVisitor );

Label l0 = new Label();
methodVisitor.visitJumpInsn( IFEQ, l0 );

stateStack.push(new If(methodVisitor, l0));
beginConditional( IFEQ );
}

@Override
public void beginIfNot( Expression test )
{
callSuperIfNecessary();
test.accept( expressionVisitor );
beginConditional( IFNE );
}

@Override
public void endBlock()
Expand Down Expand Up @@ -317,4 +316,10 @@ public void loadThis( String sourceName )
return loadsSuper[0];
}

private void beginConditional(int op)
{
Label l0 = new Label();
methodVisitor.visitJumpInsn( op, l0 );
stateStack.push(new If(methodVisitor, l0));
}
}
82 changes: 3 additions & 79 deletions community/codegen/src/main/java/org/neo4j/codegen/source/MethodSourceWriter.java
View file
Expand Up @@ -31,8 +31,6 @@
import org.neo4j.codegen.LocalVariable;
import org.neo4j.codegen.MethodEmitter;
import org.neo4j.codegen.MethodReference;
import org.neo4j.codegen.Parameter;
import org.neo4j.codegen.Resource;
import org.neo4j.codegen.TypeReference;

class MethodSourceWriter implements MethodEmitter, ExpressionVisitor
Expand Down Expand Up @@ -154,61 +152,10 @@ public void beginIf( Expression test )
level.push( LEVEL );
}

private void beginCatch( Parameter exception )
{
indent().append( "catch ( " ).append( exception.type().name() ).append( " " ).append( exception.name() )
.append( " )\n" );
indent().append( "{\n" );
level.push( LEVEL );
}

private void beginFinally()
{
indent().append( "finally\n" );
indent().append( "{\n" );
level.push( LEVEL );
}

private void beginTry( final Resource... resources )
@Override
public void beginIfNot( Expression test )
{
if ( resources.length > 0 && classSourceWriter.configuration.isSet( SourceCode.SIMPLIFY_TRY_WITH_RESOURCE ) )
{
for ( Resource resource : resources )
{
indent().append( resource.type().name() ).append( " " ).append( resource.name() ).append( " = " );
resource.producer().accept( this );
append( ";\n" );
}
indent().append( "try\n" );
indent().append( "{" );
level.push( () -> {
beginFinally();
for ( Resource resource : resources )
{
indent().append( resource.name() ).append( ".close();\n" );
}
endBlock();
} );
}
else
{
indent().append( "try" );
if ( resources.length > 0 )
{
String sep = " ( ";
for ( Resource resource : resources )
{
append( sep ).append( resource.type().name() ).append( " " ).append( resource.name() )
.append( " = " );
resource.producer().accept( this );
sep = "; ";
}
append( " )" );
}
append( "\n" );
indent().append( "{\n" );
level.push( LEVEL );
}
beginIf(Expression.not(test));
}

@Override
Expand All @@ -232,29 +179,6 @@ public <T> void tryCatchBlock( Consumer<T> body, Consumer<T> handler, LocalVaria
indent().append( "}\n" );
}

//
// indent().append( "try\n" );
// indent().append( "{\n" );
// indent();
// body.forEach( e -> e.accept( this ) );
// indent().append( '}' );
// for ( CatchClause catchClause : catchClauses )
// {
// indent().append( "catch ( " ).append( catchClause.exception().type().name() ).append( " " )
// .append( catchClause.exception().name() )
// .append( " )\n" );
// indent().append( "{\n" );
// catchClause.actions().forEach( e -> e.accept( this ) );
// }
// indent().append( '}' );
// if (!finalClauses.isEmpty())
// {
// indent().append( "finally\n" );
// indent().append( "{\n" );
// finalClauses.forEach( e -> e.accept( this ) );
// indent().append( '}' );
// }

@Override
public void throwException( Expression exception )
{
Expand Down
35 changes: 34 additions & 1 deletion community/codegen/src/test/java/org/neo4j/codegen/CodeGenerationTest.java
View file
Expand Up @@ -644,7 +644,7 @@ public void shouldGenerateIfStatement() throws Throwable
}

@Test
public void shouldGenerateIfNotStatement() throws Throwable
public void shouldGenerateIfNotExpressionStatement() throws Throwable
{
// given
ClassHandle handle;
Expand Down Expand Up @@ -676,6 +676,39 @@ public void shouldGenerateIfNotStatement() throws Throwable
verifyZeroInteractions( runner1 );
}

@Test
public void shouldGenerateIfNotStatement() throws Throwable
{
// given
ClassHandle handle;
try ( ClassGenerator simple = generateClass( "SimpleClass" ) )
{
try ( CodeBlock conditional = simple.generateMethod( void.class, "conditional",
param( boolean.class, "test" ), param( Runnable.class, "runner" ) ) )
{
try ( CodeBlock doStuff = conditional.ifNotStatement( conditional.load( "test" ) ) )
{
doStuff.expression(
invoke( doStuff.load( "runner" ), RUN ) );
}
}

handle = simple.handle();
}

Runnable runner1 = mock( Runnable.class );
Runnable runner2 = mock( Runnable.class );

// when
MethodHandle conditional = instanceMethod( handle.newInstance(), "conditional", boolean.class, Runnable.class );
conditional.invoke( true, runner1 );
conditional.invoke( false, runner2 );

// then
verify( runner2 ).run();
verifyZeroInteractions( runner1 );
}

@Test
public void shouldGenerateTryWithNestedWhileIfLoop() throws Throwable
{
Expand Down
11 changes: 6 additions & 5 deletions .../src/main/scala/org/neo4j/cypher/internal/spi/v3_1/codegen/GeneratedMethodStructure.scala
View file
Expand Up @@ -179,8 +179,9 @@ case class GeneratedMethodStructure(fields: Fields, generator: CodeBlock, aux: A
override def setInRow(column: String, value: Expression) =
generator.expression(Expression.invoke(resultRow, Methods.set, Expression.constant(column), value))

override def visitorAccept() = using(generator.ifStatement(Expression.not(
Expression.invoke(generator.load("visitor"), Methods.visit, generator.load("row"))))) { body =>
override def visitorAccept() = using(
generator.ifNotStatement(Expression.invoke(generator.load("visitor"),
Methods.visit, generator.load("row")))) { body =>
// NOTE: we are in this if-block if the visitor decided to terminate early (by returning false)
body.expression(Expression.invoke(body.self(), fields.success))
body.expression(Expression.invoke(body.self(), fields.close))
Expand Down Expand Up @@ -228,7 +229,7 @@ case class GeneratedMethodStructure(fields: Fields, generator: CodeBlock, aux: A

override def expectParameter(key: String, variableName: String) = {
using(
generator.ifStatement(Expression.not(Expression.invoke(params, Methods.mapContains, Expression.constant(key)))))
generator.ifNotStatement(Expression.invoke(params, Methods.mapContains, Expression.constant(key))))
{ block =>
block.throwException(parameterNotFoundException(key))
}
Expand Down Expand Up @@ -482,7 +483,7 @@ case class GeneratedMethodStructure(fields: Fields, generator: CodeBlock, aux: A
val list = generator.declare(hashTable.listType, context.namer.newVarName())
val elementName = context.namer.newVarName()
generator.assign(list, Expression.invoke(generator.load(tableVar), hashTable.get, generator.load(keyVar)))
using(generator.ifStatement(Expression.not(Expression.eq(list, Expression.constant(null), typeRef[Object]))))
using(generator.ifNotStatement(Expression.eq(list, Expression.constant(null), typeRef[Object])))
{ onTrue =>
using(onTrue.forEach(Parameter.param(hashTable.valueType, elementName), list)) { forEach =>
localVars.foreach {
Expand All @@ -507,7 +508,7 @@ case class GeneratedMethodStructure(fields: Fields, generator: CodeBlock, aux: A
Expression.newArray(typeRef[Long],
keyVars.map(generator.load): _*))
)))
using(generator.ifStatement(Expression.not(Expression.eq(list, Expression.constant(null), typeRef[Object]))))
using(generator.ifNotStatement(Expression.eq(list, Expression.constant(null), typeRef[Object])))
{ onTrue =>
using(onTrue.forEach(Parameter.param(hashTable.valueType, elementName), list)) { forEach =>
localVars.foreach {
Expand Down

0 comments on commit 846821a

Please sign in to comment.