From 846821a657d8ab8a1a03b80e2389ed4ac7e95305 Mon Sep 17 00:00:00 2001 From: Pontus Melke Date: Tue, 19 Apr 2016 17:59:51 +0200 Subject: [PATCH] Added ifNot as a more efficient if(not(test)) --- .../java/org/neo4j/codegen/CodeBlock.java | 7 +- .../java/org/neo4j/codegen/InvalidState.java | 6 ++ .../java/org/neo4j/codegen/MethodEmitter.java | 2 + .../bytecode/MethodByteCodeEmitter.java | 23 ++++-- .../codegen/source/MethodSourceWriter.java | 82 +------------------ .../org/neo4j/codegen/CodeGenerationTest.java | 35 +++++++- .../codegen/GeneratedMethodStructure.scala | 11 +-- 7 files changed, 71 insertions(+), 95 deletions(-) diff --git a/community/codegen/src/main/java/org/neo4j/codegen/CodeBlock.java b/community/codegen/src/main/java/org/neo4j/codegen/CodeBlock.java index 0240c12cbdadf..f23c92adabeb1 100644 --- a/community/codegen/src/main/java/org/neo4j/codegen/CodeBlock.java +++ b/community/codegen/src/main/java/org/neo4j/codegen/CodeBlock.java @@ -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 @@ -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 body, Consumer onError, Parameter exception ) { emit( e -> e.tryCatchBlock( body, onError, localVariables.createNew( exception.type(), exception.name() ), diff --git a/community/codegen/src/main/java/org/neo4j/codegen/InvalidState.java b/community/codegen/src/main/java/org/neo4j/codegen/InvalidState.java index 42967de81ca59..97cf1cdfcf3ce 100644 --- a/community/codegen/src/main/java/org/neo4j/codegen/InvalidState.java +++ b/community/codegen/src/main/java/org/neo4j/codegen/InvalidState.java @@ -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() { diff --git a/community/codegen/src/main/java/org/neo4j/codegen/MethodEmitter.java b/community/codegen/src/main/java/org/neo4j/codegen/MethodEmitter.java index 7d1d6822c14e0..445f8e69c46d4 100644 --- a/community/codegen/src/main/java/org/neo4j/codegen/MethodEmitter.java +++ b/community/codegen/src/main/java/org/neo4j/codegen/MethodEmitter.java @@ -39,6 +39,8 @@ public interface MethodEmitter void beginIf( Expression test ); + void beginIfNot( Expression test ); + void endBlock(); void tryCatchBlock( Consumer body, Consumer handler, LocalVariable exception, T block); diff --git a/community/codegen/src/main/java/org/neo4j/codegen/bytecode/MethodByteCodeEmitter.java b/community/codegen/src/main/java/org/neo4j/codegen/bytecode/MethodByteCodeEmitter.java index 6c55b89935ab2..5228948a3feb5 100644 --- a/community/codegen/src/main/java/org/neo4j/codegen/bytecode/MethodByteCodeEmitter.java +++ b/community/codegen/src/main/java/org/neo4j/codegen/bytecode/MethodByteCodeEmitter.java @@ -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; @@ -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 { @@ -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() @@ -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)); + } } diff --git a/community/codegen/src/main/java/org/neo4j/codegen/source/MethodSourceWriter.java b/community/codegen/src/main/java/org/neo4j/codegen/source/MethodSourceWriter.java index 9df9655c9a0ff..b0f79da3bb37d 100644 --- a/community/codegen/src/main/java/org/neo4j/codegen/source/MethodSourceWriter.java +++ b/community/codegen/src/main/java/org/neo4j/codegen/source/MethodSourceWriter.java @@ -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 @@ -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 @@ -232,29 +179,6 @@ public void tryCatchBlock( Consumer body, Consumer 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 ) { diff --git a/community/codegen/src/test/java/org/neo4j/codegen/CodeGenerationTest.java b/community/codegen/src/test/java/org/neo4j/codegen/CodeGenerationTest.java index b18f8613d75a3..519c7fe5510cd 100644 --- a/community/codegen/src/test/java/org/neo4j/codegen/CodeGenerationTest.java +++ b/community/codegen/src/test/java/org/neo4j/codegen/CodeGenerationTest.java @@ -644,7 +644,7 @@ public void shouldGenerateIfStatement() throws Throwable } @Test - public void shouldGenerateIfNotStatement() throws Throwable + public void shouldGenerateIfNotExpressionStatement() throws Throwable { // given ClassHandle handle; @@ -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 { diff --git a/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v3_1/codegen/GeneratedMethodStructure.scala b/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v3_1/codegen/GeneratedMethodStructure.scala index e95a5d15b8992..2e0822a2c1486 100644 --- a/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v3_1/codegen/GeneratedMethodStructure.scala +++ b/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v3_1/codegen/GeneratedMethodStructure.scala @@ -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)) @@ -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)) } @@ -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 { @@ -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 {