From 090ae57101436fdcf0d0a4243bb4a767917cbb35 Mon Sep 17 00:00:00 2001 From: Pontus Melke Date: Thu, 4 Oct 2018 08:56:17 +0200 Subject: [PATCH] Add specific option for jit compiling --- .../org/neo4j/cypher/QueryCachingTest.scala | 2 +- .../cypher/CypherExpressionEngineOption.scala | 3 ++- .../cypher/internal/ExecutionEngine.scala | 10 ++++---- .../cypher/internal/PreParsedQuery.scala | 7 +++--- .../factory/GraphDatabaseSettings.java | 2 +- .../NodeIndexContainsScanAcceptanceTest.scala | 4 ++- .../ShortestPathSameNodeAcceptanceTest.scala | 2 +- .../ExpressionEngineConfigurationTest.java | 25 ++++++++++++++----- 8 files changed, 36 insertions(+), 19 deletions(-) diff --git a/community/community-it/cypher-it/src/test/scala/org/neo4j/cypher/QueryCachingTest.scala b/community/community-it/cypher-it/src/test/scala/org/neo4j/cypher/QueryCachingTest.scala index 1053eba6c550b..ee33cb94b4da9 100644 --- a/community/community-it/cypher-it/src/test/scala/org/neo4j/cypher/QueryCachingTest.scala +++ b/community/community-it/cypher-it/src/test/scala/org/neo4j/cypher/QueryCachingTest.scala @@ -34,7 +34,7 @@ import scala.collection.{Map, mutable} class QueryCachingTest extends CypherFunSuite with GraphDatabaseTestSupport with TableDrivenPropertyChecks { - override def databaseConfig(): Map[Setting[_], String] = Map(GraphDatabaseSettings.cypher_expression_engine -> "DEFAULT", + override def databaseConfig(): Map[Setting[_], String] = Map(GraphDatabaseSettings.cypher_expression_engine -> "ONLY_WHEN_HOT", GraphDatabaseSettings.cypher_expression_recompilation_limit -> "1") test("re-uses cached plan across different execution modes") { diff --git a/community/cypher/cypher/src/main/scala/org/neo4j/cypher/CypherExpressionEngineOption.scala b/community/cypher/cypher/src/main/scala/org/neo4j/cypher/CypherExpressionEngineOption.scala index 956cfc480b7df..afe04d9fef0f3 100644 --- a/community/cypher/cypher/src/main/scala/org/neo4j/cypher/CypherExpressionEngineOption.scala +++ b/community/cypher/cypher/src/main/scala/org/neo4j/cypher/CypherExpressionEngineOption.scala @@ -25,5 +25,6 @@ case object CypherExpressionEngineOption extends CypherOptionCompanion[CypherExp case object default extends CypherExpressionEngineOption("default") case object interpreted extends CypherExpressionEngineOption("interpreted") case object compiled extends CypherExpressionEngineOption("compiled") - val all: Set[CypherExpressionEngineOption] = Set(interpreted, compiled) + case object onlyWhenHot extends CypherExpressionEngineOption("only_when_hot") + val all: Set[CypherExpressionEngineOption] = Set(interpreted, compiled, onlyWhenHot) } \ No newline at end of file diff --git a/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/ExecutionEngine.scala b/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/ExecutionEngine.scala index 6768041396537..ae9e15fd1b45b 100644 --- a/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/ExecutionEngine.scala +++ b/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/ExecutionEngine.scala @@ -124,8 +124,8 @@ class ExecutionEngine(val queryService: GraphDatabaseQueryService, tracer: QueryCompilationEvent, transactionalContext: TransactionalContext, params: MapValue): (() => ExecutableQuery, (Int) => Option[ExecutableQuery]) = preParsedQuery.expressionEngine match { - //the default is to start with interpreted and change to compiled when hot enough - case CypherExpressionEngineOption.default if config.recompilationLimit > 0 => + //check if we need to jit compiling of queries + case CypherExpressionEngineOption.onlyWhenHot if config.recompilationLimit > 0 => val primary: () => ExecutableQuery = () => masterCompiler.compile(preParsedQuery, tracer, transactionalContext, params) val secondary: (Int) => Option[ExecutableQuery] = @@ -137,11 +137,11 @@ class ExecutionEngine(val queryService: GraphDatabaseQueryService, (primary, secondary) //We have recompilationLimit == 0, go to compiled directly - case CypherExpressionEngineOption.default => - (() => masterCompiler.compile(preParsedQuery.copy(expressionEngine = CypherExpressionEngineOption.compiled), + case CypherExpressionEngineOption.onlyWhenHot => + (() => masterCompiler.compile(preParsedQuery.copy(recompilationLimitReached = true), tracer, transactionalContext, params), (_) => None) //In the other cases we have no recompilation step - case _ => (() => masterCompiler.compile(preParsedQuery,tracer, transactionalContext, params), (_) => None) + case _ => (() => masterCompiler.compile(preParsedQuery, tracer, transactionalContext, params), (_) => None) } private def getOrCompile(context: TransactionalContext, diff --git a/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/PreParsedQuery.scala b/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/PreParsedQuery.scala index 298bcd7bbf705..6726c121f1e61 100644 --- a/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/PreParsedQuery.scala +++ b/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/PreParsedQuery.scala @@ -53,8 +53,8 @@ case class PreParsedQuery(statement: String, } val expressionEngineInfo = expressionEngine match { - case CypherExpressionEngineOption.compiled => s"expressionEngine=${expressionEngine.name}" - case _ => "" + case CypherExpressionEngineOption.default | CypherExpressionEngineOption.onlyWhenHot => "" + case _ => s"expressionEngine=${expressionEngine.name}" } val debugFlags = debugOptions.map(flag => s"debug=$flag").mkString(" ") @@ -62,5 +62,6 @@ case class PreParsedQuery(statement: String, s"CYPHER ${version.name} $plannerInfo $runtimeInfo $updateStrategyInfo $expressionEngineInfo $debugFlags $statement" } - def useCompiledExpressions: Boolean = expressionEngine == CypherExpressionEngineOption.compiled || recompilationLimitReached + def useCompiledExpressions: Boolean = expressionEngine == CypherExpressionEngineOption.compiled || + (expressionEngine == CypherExpressionEngineOption.onlyWhenHot && recompilationLimitReached) } diff --git a/community/kernel/src/main/java/org/neo4j/graphdb/factory/GraphDatabaseSettings.java b/community/kernel/src/main/java/org/neo4j/graphdb/factory/GraphDatabaseSettings.java index 6adb7fccf0ebf..ea72208cb81a4 100644 --- a/community/kernel/src/main/java/org/neo4j/graphdb/factory/GraphDatabaseSettings.java +++ b/community/kernel/src/main/java/org/neo4j/graphdb/factory/GraphDatabaseSettings.java @@ -246,7 +246,7 @@ public class GraphDatabaseSettings implements LoadableConfig "never be compiled." ) @Internal public static final Setting cypher_expression_engine = setting( - "unsupported.cypher.expression_engine", optionsIgnoreCase( "INTERPRETED", "COMPILED", DEFAULT ), "INTERPRETED" ); + "unsupported.cypher.expression_engine", optionsIgnoreCase( "INTERPRETED", "COMPILED", "ONLY_WHEN_HOT", DEFAULT ), DEFAULT ); @Description( "Number of uses before an expression is considered for compilation" ) @Internal diff --git a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/NodeIndexContainsScanAcceptanceTest.scala b/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/NodeIndexContainsScanAcceptanceTest.scala index 517526bf9adcb..e2e3f4bdbde34 100644 --- a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/NodeIndexContainsScanAcceptanceTest.scala +++ b/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/NodeIndexContainsScanAcceptanceTest.scala @@ -186,7 +186,9 @@ class NodeIndexContainsScanAcceptanceTest extends ExecutionEngineFunSuite with C val query = "MATCH (l:Location) WHERE l.name CONTAINS {param} RETURN l" - failWithError(TestConfiguration(Versions.Default, Planners.Default, Runtimes(Runtimes.ProcedureOrSchema, Runtimes.Interpreted, Runtimes.Slotted)) + + failWithError(TestConfiguration(Versions.Default, Planners.Default, Runtimes(Runtimes.ProcedureOrSchema, + Runtimes.Interpreted, Runtimes.Slotted, + Runtimes.SlottedWithCompiledExpressions)) + TestConfiguration(Versions.all, Planners.Cost, Runtimes(Runtimes.Interpreted, Runtimes.Default)) + TestConfiguration(Versions.V2_3, Planners.Rule, Runtimes(Runtimes.Interpreted, Runtimes.Default)), query, message = List("Expected a string value, but got 42","Expected a string value, but got Int(42)","Expected two strings, but got London and 42"), diff --git a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/ShortestPathSameNodeAcceptanceTest.scala b/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/ShortestPathSameNodeAcceptanceTest.scala index 71d59ff0ef1e0..b11d1c9430763 100644 --- a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/ShortestPathSameNodeAcceptanceTest.scala +++ b/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/ShortestPathSameNodeAcceptanceTest.scala @@ -37,7 +37,7 @@ class ShortestPathSameNodeAcceptanceTest extends ExecutionEngineFunSuite with Ru val expectedToFail = TestConfiguration( Versions(Versions.Default, V3_1, V3_4), Planners(Planners.Cost, Planners.Rule, Planners.Default), - Runtimes(Runtimes.Interpreted, Runtimes.Slotted, Runtimes.Default, Runtimes.ProcedureOrSchema)) + Runtimes(Runtimes.Interpreted, Runtimes.Slotted, Runtimes.SlottedWithCompiledExpressions, Runtimes.Default, Runtimes.ProcedureOrSchema)) def setupModel(db: GraphDatabaseCypherService) { db.inTx { diff --git a/enterprise/cypher/cypher/src/test/java/org/neo4j/cypher/internal/javacompat/ExpressionEngineConfigurationTest.java b/enterprise/cypher/cypher/src/test/java/org/neo4j/cypher/internal/javacompat/ExpressionEngineConfigurationTest.java index 75ad72ae826cd..d1d04cb2803e1 100644 --- a/enterprise/cypher/cypher/src/test/java/org/neo4j/cypher/internal/javacompat/ExpressionEngineConfigurationTest.java +++ b/enterprise/cypher/cypher/src/test/java/org/neo4j/cypher/internal/javacompat/ExpressionEngineConfigurationTest.java @@ -39,23 +39,36 @@ class ExpressionEngineConfigurationTest private final AssertableLogProvider logProvider = new AssertableLogProvider(); @Test - void shouldNotUseCompiledExpressionsFirstTimeWithDefaultSettings() + void shouldBeUsingInterpretedByDefault() { - assertNotUsingCompiled( withEngineAndLimit( "DEFAULT", 1 ), "RETURN sin(cos(sin(cos(rand()))))" ); + // Given + String query = "RETURN sin(cos(sin(cos(rand()))))"; + GraphDatabaseService db = withEngineAndLimit( "DEFAULT", 0 ); + int manyTimes = 10; + for ( int i = 0; i < manyTimes; i++ ) + { + assertNotUsingCompiled( db, query ); + } + } + + @Test + void shouldNotUseCompiledExpressionsFirstTimeWithJitEnabled() + { + assertNotUsingCompiled( withEngineAndLimit( "ONLY_WHEN_HOT", 1 ), "RETURN sin(cos(sin(cos(rand()))))" ); } @Test void shouldUseCompiledExpressionsFirstTimeWhenLimitIsZero() { - assertUsingCompiled( withEngineAndLimit( "DEFAULT", 0 ), "RETURN sin(cos(sin(cos(rand()))))" ); + assertUsingCompiled( withEngineAndLimit( "ONLY_WHEN_HOT", 0 ), "RETURN sin(cos(sin(cos(rand()))))" ); } @Test - void shouldUseCompiledExpressionsWhenQueryIsHotWithDefaultSettings() + void shouldUseCompiledExpressionsWhenQueryIsHotWithJitEnabled() { // Given String query = "RETURN sin(cos(sin(cos(rand()))))"; - GraphDatabaseService db = withEngineAndLimit( "DEFAULT", 3 ); + GraphDatabaseService db = withEngineAndLimit( "ONLY_WHEN_HOT", 3 ); // When db.execute( query ); @@ -75,7 +88,7 @@ void shouldUseCompiledExpressionsFirstTimeWhenConfigured() @Test void shouldUseCompiledExpressionsFirstTimeWhenExplicitlyAskedFor() { - assertUsingCompiled( withEngineAndLimit( "DEFAULT", 42 ), + assertUsingCompiled( withEngineAndLimit( "ONLY_WHEN_HOT", 42 ), "CYPHER expressionEngine=COMPILED RETURN sin(cos(sin(cos(rand()))))" ); }