From 3c702c065526f2d35e7dfff01afe2655b4110b09 Mon Sep 17 00:00:00 2001 From: Edmund Higham Date: Tue, 8 Oct 2024 14:49:06 -0400 Subject: [PATCH] review feedback from @patrick-schultz --- hail/src/main/scala/is/hail/backend/ExecuteContext.scala | 2 +- .../main/scala/is/hail/backend/local/LocalBackend.scala | 5 +++-- .../scala/is/hail/backend/service/ServiceBackend.scala | 5 +++-- .../main/scala/is/hail/backend/spark/SparkBackend.scala | 7 ++++--- hail/src/main/scala/is/hail/expr/ir/Emit.scala | 9 ++------- .../scala/is/hail/expr/ir/lowering/LoweringPass.scala | 4 +++- .../main/scala/is/hail/expr/ir/streams/EmitStream.scala | 2 +- 7 files changed, 17 insertions(+), 17 deletions(-) diff --git a/hail/src/main/scala/is/hail/backend/ExecuteContext.scala b/hail/src/main/scala/is/hail/backend/ExecuteContext.scala index 2ed4ab9d456..1e692b4e48c 100644 --- a/hail/src/main/scala/is/hail/backend/ExecuteContext.scala +++ b/hail/src/main/scala/is/hail/backend/ExecuteContext.scala @@ -114,7 +114,7 @@ class ExecuteContext( val theHailClassLoader: HailClassLoader, val flags: HailFeatureFlags, val backendContext: BackendContext, - var irMetadata: IrMetadata, + val irMetadata: IrMetadata, ) extends Closeable { val rngNonce: Long = diff --git a/hail/src/main/scala/is/hail/backend/local/LocalBackend.scala b/hail/src/main/scala/is/hail/backend/local/LocalBackend.scala index b6a86cf7ce7..48baa95516d 100644 --- a/hail/src/main/scala/is/hail/backend/local/LocalBackend.scala +++ b/hail/src/main/scala/is/hail/backend/local/LocalBackend.scala @@ -96,7 +96,7 @@ class LocalBackend(val tmpdir: String) extends Backend with BackendWithCodeCache override val executionCache: ExecutionCache = ExecutionCache.fromFlags(flags, fs, tmpdir) }, - IrMetadata(None), + new IrMetadata(), )(f) } @@ -182,9 +182,10 @@ class LocalBackend(val tmpdir: String) extends Backend with BackendWithCodeCache ctx.time { TypeCheck(ctx, ir) Validate(ir) + assert(ir.typ.isRealizable) val queryID = Backend.nextID() log.info(s"starting execution of query $queryID of initial size ${IRSize(ir)}") - ctx.irMetadata = ctx.irMetadata.copy(semhash = SemanticHash(ctx)(ir)) + ctx.irMetadata.semhash = SemanticHash(ctx)(ir) val res = _jvmLowerAndExecute(ctx, ir) log.info(s"finished execution of query $queryID") res diff --git a/hail/src/main/scala/is/hail/backend/service/ServiceBackend.scala b/hail/src/main/scala/is/hail/backend/service/ServiceBackend.scala index 00c977b55aa..779d8b7c15e 100644 --- a/hail/src/main/scala/is/hail/backend/service/ServiceBackend.scala +++ b/hail/src/main/scala/is/hail/backend/service/ServiceBackend.scala @@ -335,9 +335,10 @@ class ServiceBackend( ctx.time { TypeCheck(ctx, ir) Validate(ir) + assert(ir.typ.isRealizable) val queryID = Backend.nextID() log.info(s"starting execution of query $queryID of initial size ${IRSize(ir)}") - ctx.irMetadata = ctx.irMetadata.copy(semhash = SemanticHash(ctx)(ir)) + ctx.irMetadata.semhash = SemanticHash(ctx)(ir) val res = _jvmLowerAndExecute(ctx, ir) log.info(s"finished execution of query $queryID") res @@ -408,7 +409,7 @@ class ServiceBackend( theHailClassLoader, flags, serviceBackendContext, - IrMetadata(None), + new IrMetadata(), )(f) } diff --git a/hail/src/main/scala/is/hail/backend/spark/SparkBackend.scala b/hail/src/main/scala/is/hail/backend/spark/SparkBackend.scala index c6955a48cb9..e9299a420b1 100644 --- a/hail/src/main/scala/is/hail/backend/spark/SparkBackend.scala +++ b/hail/src/main/scala/is/hail/backend/spark/SparkBackend.scala @@ -372,7 +372,7 @@ class SparkBackend( override val executionCache: ExecutionCache = ExecutionCache.forTesting }, - IrMetadata(None), + new IrMetadata(), ) def withExecuteContext[T]( @@ -408,7 +408,7 @@ class SparkBackend( override val executionCache: ExecutionCache = ExecutionCache.fromFlags(flags, fs, tmpdir) }, - IrMetadata(None), + new IrMetadata(), )(f) } @@ -542,7 +542,8 @@ class SparkBackend( ctx.time { TypeCheck(ctx, ir) Validate(ir) - ctx.irMetadata = ctx.irMetadata.copy(semhash = SemanticHash(ctx)(ir)) + assert(ir.typ.isRealizable) + ctx.irMetadata.semhash = SemanticHash(ctx)(ir) try { val lowerTable = getFlag("lower") != null val lowerBM = getFlag("lower_bm") != null diff --git a/hail/src/main/scala/is/hail/expr/ir/Emit.scala b/hail/src/main/scala/is/hail/expr/ir/Emit.scala index 1c141d3bdbb..f52325c6f23 100644 --- a/hail/src/main/scala/is/hail/expr/ir/Emit.scala +++ b/hail/src/main/scala/is/hail/expr/ir/Emit.scala @@ -29,8 +29,6 @@ import scala.language.existentials import java.io._ -import sourcecode.Enclosing - // class for holding all information computed ahead-of-time that we need in the emitter object EmitContext { def analyze(ctx: ExecuteContext, ir: IR, pTypeEnv: Env[PType] = Env.empty): EmitContext = { @@ -58,10 +56,7 @@ case class EmitContext( methodSplits: Memo[Unit], inLoopCriticalPath: Memo[Unit], tryingToSplit: Memo[Unit], -) { - def time[A](block: => A)(implicit E: Enclosing): A = - executeContext.time(block) -} +) case class EmitEnv(bindings: Env[EmitValue], inputValues: IndexedSeq[EmitValue]) { def bind(name: Name, v: EmitValue): EmitEnv = copy(bindings = bindings.bind(name, v)) @@ -106,7 +101,7 @@ object Emit { nParams: Int, aggs: Option[Array[AggStateSig]] = None, ): Option[SingleCodeType] = - ctx.time { + ctx.executeContext.time { TypeCheck(ctx.executeContext, ir) val mb = fb.apply_method diff --git a/hail/src/main/scala/is/hail/expr/ir/lowering/LoweringPass.scala b/hail/src/main/scala/is/hail/expr/ir/lowering/LoweringPass.scala index c7ac8f8b39f..f821e58da24 100644 --- a/hail/src/main/scala/is/hail/expr/ir/lowering/LoweringPass.scala +++ b/hail/src/main/scala/is/hail/expr/ir/lowering/LoweringPass.scala @@ -6,10 +6,12 @@ import is.hail.expr.ir.agg.Extract import is.hail.expr.ir.analyses.SemanticHash import is.hail.utils._ -final case class IrMetadata(semhash: Option[SemanticHash.Type]) { +final class IrMetadata() { private[this] var hashCounter: Int = 0 private[this] var markCounter: Int = 0 + var semhash: Option[SemanticHash.Type] = None + def nextHash: Option[SemanticHash.Type] = { hashCounter += 1 semhash.map(SemanticHash.extend(_, SemanticHash.Bytes.fromInt(hashCounter))) diff --git a/hail/src/main/scala/is/hail/expr/ir/streams/EmitStream.scala b/hail/src/main/scala/is/hail/expr/ir/streams/EmitStream.scala index 17b5059daf5..d8ff0f7cf6f 100644 --- a/hail/src/main/scala/is/hail/expr/ir/streams/EmitStream.scala +++ b/hail/src/main/scala/is/hail/expr/ir/streams/EmitStream.scala @@ -145,7 +145,7 @@ object EmitStream { env: EmitEnv, container: Option[AggContainer], ): IEmitCode = - emitter.ctx.time { + emitter.ctx.executeContext.time { @nowarn("cat=unused-locals&msg=local default argument") def emitVoid(