From 7dc58d42b6905a7fd7531875fa76060f8f744e4e Mon Sep 17 00:00:00 2001 From: Tanguy Date: Fri, 10 Dec 2021 11:19:14 +0100 Subject: [PATCH] Improve ram usage (#243) Remove cyclic references of {.async.} Futures, allowing them to be picked up by the regular refc instead of Mark and Sweep --- chronos/asyncfutures2.nim | 48 ++++++++++- chronos/asyncmacro2.nim | 172 ++++++++++++-------------------------- 2 files changed, 99 insertions(+), 121 deletions(-) diff --git a/chronos/asyncfutures2.nim b/chronos/asyncfutures2.nim index bbecdfa7..b873292e 100644 --- a/chronos/asyncfutures2.nim +++ b/chronos/asyncfutures2.nim @@ -54,6 +54,10 @@ type # How much refactoring is needed to make this a regular non-ref type? # Obviously, it will still be allocated on the heap when necessary. Future*[T] = ref object of FutureBase ## Typed future. + when defined(chronosStrictException): + closure*: iterator(f: Future[T]): FutureBase {.raises: [Defect, CatchableError], gcsafe.} + else: + closure*: iterator(f: Future[T]): FutureBase {.raises: [Defect, CatchableError, Exception], gcsafe.} value: T ## Stored value FutureStr*[T] = ref object of Future[T] @@ -351,6 +355,46 @@ proc `cancelCallback=`*[T](future: Future[T], cb: CallbackFunc) = ## This callback will be called immediately as ``future.cancel()`` invoked. future.cancelcb = cb +{.push stackTrace: off.} +proc internalContinue[T](fut: pointer) {.gcsafe, raises: [Defect].} + +proc futureContinue*[T](fut: Future[T]) {.gcsafe, raises: [Defect].} = + # Used internally by async transformation + try: + if not(fut.closure.finished()): + var next = fut.closure(fut) + # Continue while the yielded future is already finished. + while (not next.isNil()) and next.finished(): + next = fut.closure(fut) + if fut.closure.finished(): + break + + if fut.closure.finished(): + fut.closure = nil + if next == nil: + if not(fut.finished()): + raiseAssert "Async procedure (" & ($fut.location[LocCreateIndex]) & ") yielded `nil`, " & + "are you await'ing a `nil` Future?" + else: + GC_ref(fut) + next.addCallback(internalContinue[T], cast[pointer](fut)) + except CancelledError: + fut.cancelAndSchedule() + except CatchableError as exc: + fut.fail(exc) + except Exception as exc: + if exc of Defect: + raise (ref Defect)(exc) + + fut.fail((ref ValueError)(msg: exc.msg, parent: exc)) + +proc internalContinue[T](fut: pointer) {.gcsafe, raises: [Defect].} = + let asFut = cast[Future[T]](fut) + GC_unref(asFut) + futureContinue(asFut) + +{.pop.} + template getFilenameProcname(entry: StackTraceEntry): (string, string) = when compiles(entry.filenameStr) and compiles(entry.procnameStr): # We can't rely on "entry.filename" and "entry.procname" still being valid @@ -375,8 +419,8 @@ proc getHint(entry: StackTraceEntry): string = if cmpIgnoreStyle(filename, "asyncdispatch.nim") == 0: return "Processes asynchronous completion events" - if procname.endsWith("_continue"): - if cmpIgnoreStyle(filename, "asyncmacro.nim") == 0: + if procname == "internalContinue": + if cmpIgnoreStyle(filename, "asyncfutures.nim") == 0: return "Resumes an async procedure" proc `$`(stackTraceEntries: seq[StackTraceEntry]): string = diff --git a/chronos/asyncmacro2.nim b/chronos/asyncmacro2.nim index c30b9786..04fda6ac 100644 --- a/chronos/asyncmacro2.nim +++ b/chronos/asyncmacro2.nim @@ -15,89 +15,6 @@ proc skipUntilStmtList(node: NimNode): NimNode {.compileTime.} = if node[0].kind == nnkStmtList: result = skipUntilStmtList(node[0]) -# proc skipStmtList(node: NimNode): NimNode {.compileTime.} = -# result = node -# if node[0].kind == nnkStmtList: -# result = node[0] -when defined(chronosStrictException): - template createCb(retFutureSym, iteratorNameSym, - strName, identName: untyped) = - bind finished - - var nameIterVar = iteratorNameSym - {.push stackTrace: off.} - var identName: proc(udata: pointer) {.gcsafe, raises: [Defect].} - identName = proc(udata: pointer) {.gcsafe, raises: [Defect].} = - try: - # If the compiler complains about unlisted exception here, it's usually - # because you're calling a callback or forward declaration in your code - # for which the compiler cannot deduce raises signatures - make sure - # to annotate both forward declarations and `proc` types with `raises`! - if not(nameIterVar.finished()): - var next = nameIterVar() - # Continue while the yielded future is already finished. - while (not next.isNil()) and next.finished(): - next = nameIterVar() - if nameIterVar.finished(): - break - - if next == nil: - if not(retFutureSym.finished()): - const msg = "Async procedure (&" & strName & ") yielded `nil`, " & - "are you await'ing a `nil` Future?" - raiseAssert msg - else: - next.addCallback(identName) - except CancelledError: - retFutureSym.cancelAndSchedule() - except CatchableError as exc: - retFutureSym.fail(exc) - - identName(nil) - {.pop.} -else: - template createCb(retFutureSym, iteratorNameSym, - strName, identName: untyped) = - bind finished - - var nameIterVar = iteratorNameSym - {.push stackTrace: off.} - var identName: proc(udata: pointer) {.gcsafe, raises: [Defect].} - identName = proc(udata: pointer) {.gcsafe, raises: [Defect].} = - try: - # If the compiler complains about unlisted exception here, it's usually - # because you're calling a callback or forward declaration in your code - # for which the compiler cannot deduce raises signatures - make sure - # to annotate both forward declarations and `proc` types with `raises`! - if not(nameIterVar.finished()): - var next = nameIterVar() - # Continue while the yielded future is already finished. - while (not next.isNil()) and next.finished(): - next = nameIterVar() - if nameIterVar.finished(): - break - - if next == nil: - if not(retFutureSym.finished()): - const msg = "Async procedure (&" & strName & ") yielded `nil`, " & - "are you await'ing a `nil` Future?" - raiseAssert msg - else: - next.addCallback(identName) - except CancelledError: - retFutureSym.cancelAndSchedule() - except CatchableError as exc: - retFutureSym.fail(exc) - except Exception as exc: - # TODO remove Exception handler to turn on strict mode - if exc of Defect: - raise (ref Defect)(exc) - - retFutureSym.fail((ref ValueError)(msg: exc.msg, parent: exc)) - - identName(nil) - {.pop.} - proc processBody(node, retFutureSym: NimNode, subTypeIsVoid: bool): NimNode {.compileTime.} = #echo(node.treeRepr) @@ -185,31 +102,16 @@ proc asyncSingleProc(prc: NimNode): NimNode {.compileTime.} = var outerProcBody = newNimNode(nnkStmtList, prc.body) - # -> var retFuture = newFuture[T]() - var retFutureSym = ident "chronosInternalRetFuture" - var subRetType = - if returnType.kind == nnkEmpty: - newIdentNode("void") - else: - baseType - # Do not change this code to `quote do` version because `instantiationInfo` - # will be broken for `newFuture()` call. - outerProcBody.add( - newVarStmt( - retFutureSym, - newCall(newTree(nnkBracketExpr, ident "newFuture", subRetType), - newLit(prcName)) - ) - ) - # -> iterator nameIter(): FutureBase {.closure.} = + # -> iterator nameIter(chronosInternalRetFuture: Future[T]): FutureBase {.closure.} = # -> {.push warning[resultshadowed]: off.} # -> var result: T # -> {.pop.} # -> - # -> complete(retFuture, result) + # -> complete(chronosInternalRetFuture, result) + let internalFutureSym = ident "chronosInternalRetFuture" var iteratorNameSym = genSym(nskIterator, $prcName) - var procBody = prc.body.processBody(retFutureSym, subtypeIsVoid) + var procBody = prc.body.processBody(internalFutureSym, subtypeIsVoid) # don't do anything with forward bodies (empty) if procBody.kind != nnkEmpty: if subtypeIsVoid: @@ -236,12 +138,18 @@ proc asyncSingleProc(prc: NimNode): NimNode {.compileTime.} = procBody.add( newCall(newIdentNode("complete"), - retFutureSym, newIdentNode("result"))) # -> complete(retFuture, result) + internalFutureSym, newIdentNode("result"))) # -> complete(chronosInternalRetFuture, result) else: - # -> complete(retFuture) - procBody.add(newCall(newIdentNode("complete"), retFutureSym)) + # -> complete(chronosInternalRetFuture) + procBody.add(newCall(newIdentNode("complete"), internalFutureSym)) - var closureIterator = newProc(iteratorNameSym, [newIdentNode("FutureBase")], + let + internalFutureType = + if subtypeIsVoid: + newNimNode(nnkBracketExpr, prc).add(newIdentNode("Future")).add(newIdentNode("void")) + else: returnType + internalFutureParameter = nnkIdentDefs.newTree(internalFutureSym, internalFutureType, newEmptyNode()) + var closureIterator = newProc(iteratorNameSym, [newIdentNode("FutureBase"), internalFutureParameter], procBody, nnkIteratorDef) closureIterator.pragma = newNimNode(nnkPragma, lineInfoFrom=prc.body) closureIterator.addPragma(newIdentNode("closure")) @@ -284,16 +192,38 @@ proc asyncSingleProc(prc: NimNode): NimNode {.compileTime.} = closureIterator.addPragma(newIdentNode("gcsafe")) outerProcBody.add(closureIterator) - # -> createCb(retFuture) - # NOTE: The "_continue" suffix is checked for in asyncfutures.nim to produce - # friendlier stack traces: - var cbName = genSym(nskVar, prcName & "_continue") - var procCb = getAst createCb(retFutureSym, iteratorNameSym, - newStrLitNode(prcName), - cbName) - outerProcBody.add procCb + # -> var resultFuture = newFuture[T]() + # declared at the end to be sure that the closure + # doesn't reference it, avoid cyclic ref (#203) + var retFutureSym = ident "resultFuture" + var subRetType = + if returnType.kind == nnkEmpty: + newIdentNode("void") + else: + baseType + # Do not change this code to `quote do` version because `instantiationInfo` + # will be broken for `newFuture()` call. + outerProcBody.add( + newVarStmt( + retFutureSym, + newCall(newTree(nnkBracketExpr, ident "newFuture", subRetType), + newLit(prcName)) + ) + ) + + # -> resultFuture.closure = iterator + outerProcBody.add( + newAssignment( + newDotExpr(retFutureSym, newIdentNode("closure")), + iteratorNameSym) + ) - # -> return retFuture + # -> futureContinue(resultFuture)) + outerProcBody.add( + newCall(newIdentNode("futureContinue"), retFutureSym) + ) + + # -> return resultFuture outerProcBody.add newNimNode(nnkReturnStmt, prc.body[^1]).add(retFutureSym) if prc.kind != nnkLambda: # TODO: Nim bug? @@ -320,9 +250,11 @@ proc asyncSingleProc(prc: NimNode): NimNode {.compileTime.} = template await*[T](f: Future[T]): untyped = when declared(chronosInternalRetFuture): + #work around https://github.com/nim-lang/Nim/issues/19193 when not declaredInScope(chronosInternalTmpFuture): - var chronosInternalTmpFuture {.inject.}: FutureBase - chronosInternalTmpFuture = f + var chronosInternalTmpFuture {.inject.}: FutureBase = f + else: + chronosInternalTmpFuture = f chronosInternalRetFuture.child = chronosInternalTmpFuture # This "yield" is meant for a closure iterator in the caller. @@ -348,9 +280,11 @@ template await*[T](f: Future[T]): untyped = template awaitne*[T](f: Future[T]): Future[T] = when declared(chronosInternalRetFuture): + #work around https://github.com/nim-lang/Nim/issues/19193 when not declaredInScope(chronosInternalTmpFuture): - var chronosInternalTmpFuture {.inject.}: FutureBase - chronosInternalTmpFuture = f + var chronosInternalTmpFuture {.inject.}: FutureBase = f + else: + chronosInternalTmpFuture = f chronosInternalRetFuture.child = chronosInternalTmpFuture yield chronosInternalTmpFuture chronosInternalRetFuture.child = nil