From 05a7a9fa5af70f218b6165a5817c0e36243f7d0b Mon Sep 17 00:00:00 2001 From: Gabriel Cruz Date: Mon, 6 Jul 2026 11:10:54 -0300 Subject: [PATCH] fix: pr comments --- ffi/codegen/meta.nim | 47 -------- ffi/ffi_thread_request.nim | 5 - ffi/internal/ffi_macro.nim | 114 +++--------------- ffi/internal/ffi_scalar.nim | 172 ++++++++++++++++++++++++++++ tests/unit/test_scalar_fastpath.nim | 1 + 5 files changed, 191 insertions(+), 148 deletions(-) create mode 100644 ffi/internal/ffi_scalar.nim diff --git a/ffi/codegen/meta.nim b/ffi/codegen/meta.nim index e6e7143..eb60784 100644 --- a/ffi/codegen/meta.nim +++ b/ffi/codegen/meta.nim @@ -67,53 +67,6 @@ var libraryDeclared* {.compileTime.}: bool = false # Library-wide default ABI, inherited by each annotation unless it overrides. var currentDefaultABIFormat* {.compileTime.}: ABIFormat = ABIFormat.Cbor -const scalarPodTypeNames = [ - "int", "int8", "int16", "int32", "int64", "uint", "uint8", "uint16", "uint32", - "uint64", "byte", "float", "float32", "float64", "bool", -] - ## Fixed-width POD scalars that fit a single `uint64` slot and survive the - ## async hop by value — the payload the scalar fast path inlines into the - ## request (no heap copy). `cstring`/`string` are intentionally absent as - ## *params*: they are pointers to caller memory the FFI thread reads later, - ## so they'd need a copy, defeating the zero-alloc promise. - -func isScalarParamTypeName*(name: string): bool = - ## A param type eligible for the CBOR-free scalar fast path. - name in scalarPodTypeNames - -func isScalarReturnTypeName*(name: string): bool = - ## A return type eligible for the scalar fast path. Unlike params, a - ## `string`/`cstring` return is fine: the handler produces the bytes and they - ## ride back raw (like the error path), so no caller memory is aliased. - name in scalarPodTypeNames or name == "string" or name == "cstring" - -func isScalarOnly*(p: FFIProcMeta): bool = - ## True iff `p` is a plain `{.ffi.}` method whose every wire param and return - ## is scalar — the whole signature crosses without CBOR or `_CWire`. Handles - ## and raw pointers are excluded (a handle needs a ctx-registry round-trip; - ## a pointer never crosses). Pure over the compile-time metadata. - if p.kind != FFIKind.FFI: - return false - if p.returnIsPtr or p.returnIsHandle: - return false - if not isScalarReturnTypeName(p.returnTypeName): - return false - for ep in p.extraParams: - if ep.isPtr or ep.isHandle or not isScalarParamTypeName(ep.typeName): - return false - true - -func bindableProcs*(procs: seq[FFIProcMeta]): seq[FFIProcMeta] = - ## The procs the foreign-binding generators emit for. Scalar-fast-path procs - ## are dropped: their C export takes inline scalar args, not the CBOR - ## `(reqCbor, reqCborLen)` shape the current codegen assumes, so emitting a - ## CBOR caller for them would be wrong. Foreign codegen is a follow-up. - var kept: seq[FFIProcMeta] = @[] - for p in procs: - if not p.scalarFastPath: - kept.add(p) - kept - proc abiCodegenImplemented*(fmt: ABIFormat): bool = ## Whether `fmt` has a working proc-dispatch path. Only `Cbor` does today; the ## seam a future PR flips once the `c` dispatch path is wired. diff --git a/ffi/ffi_thread_request.nim b/ffi/ffi_thread_request.nim index 420fb71..4cbdc8d 100644 --- a/ffi/ffi_thread_request.nim +++ b/ffi/ffi_thread_request.nim @@ -30,9 +30,6 @@ type FFIThreadRequest* = object reqId*: cstring ## Per-proc Req type name used to look up the handler. data*: ptr UncheckedArray[byte] ## Owned CBOR-encoded request payload. dataLen*: int - isScalar*: bool - ## Set by `initScalar`: the payload rode inline in `scalarArgs` (no CBOR, - ## no `data` buffer), so `deleteRequest` has nothing extra to free. scalarArgs*: array[MaxScalarArgs, uint64] ## Scalar-fast-path args inlined in the envelope (one `ffiPackScalar` value ## per slot) so there's no per-call `c_malloc`. A plain array rather than a @@ -79,7 +76,6 @@ proc allocBaseRequest( ret[].reqId = reqId.alloc() ret[].data = nil ret[].dataLen = 0 - ret[].isScalar = false ret[].next = nil return ret @@ -169,7 +165,6 @@ proc initScalar*( "initScalar: " & $args.len & " scalar args exceed MaxScalarArgs (" & $MaxScalarArgs & ")" var ret = allocBaseRequest(callback, userData, reqId) - ret[].isScalar = true for i in 0 ..< args.len: ret[].scalarArgs[i] = args[i] ret diff --git a/ffi/internal/ffi_macro.nim b/ffi/internal/ffi_macro.nim index af931dc..da7defd 100644 --- a/ffi/internal/ffi_macro.nim +++ b/ffi/internal/ffi_macro.nim @@ -4,6 +4,7 @@ import ../ffi_types import ../ffi_thread_request import ../codegen/[meta, string_helpers] import ./c_macro_helpers +import ./ffi_scalar when defined(ffiGenBindings): import ../codegen/rust import ../codegen/cpp @@ -1013,103 +1014,24 @@ macro ffi*(args: varargs[untyped]): untyped = return newStmtList(helperProc, registerReq, ffiProc) proc scalarPath(): NimNode = - ## CBOR-free dispatch for an all-scalar `.ffi.` method. The C export packs - ## its scalar args inline into the request (no envelope `c_malloc`, no CBOR - ## encode); the FFI-thread handler unpacks them, runs the user body, and - ## returns the result as raw bytes (see `ffiScalarRetBytes`). The Nim-facing - ## helper is emitted unchanged so the proc stays directly callable from Nim. - let helperProc = buildAsyncHelperProc() - - let ptrFFICtx = - nnkPtrTy.newTree(nnkBracketExpr.newTree(ident("FFIContext"), libTypeName)) - let scalarReqKey = camelName & "Req" - - let reqIdent = genSym(nskLet, "ffiReq") - let ctxHandlerName = genSym(nskLet, "ffiCtxHandler") - let handlerBody = newStmtList() - handlerBody.add quote do: - let `reqIdent` = cast[ptr FFIThreadRequest](request) - let `ctxHandlerName` = cast[`ptrFFICtx`](reqHandler) - - let helperCall = newTree(nnkCall, userProcName) - let ctxMyLib = newDotExpr(newTree(nnkDerefExpr, ctxHandlerName), ident("myLib")) - helperCall.add(newTree(nnkDerefExpr, ctxMyLib)) - for i in 0 ..< extraParamNames.len: - let argIdent = ident(extraParamNames[i]) - let slot = nnkBracketExpr.newTree( - newDotExpr(newTree(nnkDerefExpr, reqIdent), ident("scalarArgs")), newLit(i) - ) - handlerBody.add( - newLetStmt( - argIdent, newCall(ident("ffiUnpackScalar"), slot, extraParamTypes[i]) - ) - ) - helperCall.add(argIdent) - - let retValIdent = genSym(nskLet, "retVal") - handlerBody.add quote do: - let `retValIdent` = (await `helperCall`).valueOr: - return err($error) - return ok(ffiScalarRetBytes(`retValIdent`)) - - let seqByteResult = nnkBracketExpr.newTree( - ident("Future"), - nnkBracketExpr.newTree( - ident("Result"), - nnkBracketExpr.newTree(ident("seq"), ident("byte")), - ident("string"), - ), - ) - let handlerProc = newProc( - name = newEmptyNode(), - params = @[ - seqByteResult, - newIdentDefs(ident("request"), ident("pointer")), - newIdentDefs(ident("reqHandler"), ident("pointer")), - ], - body = handlerBody, - pragmas = nnkPragma.newTree(ident("async")), - ) - let registerAssign = newAssignment( - nnkBracketExpr.newTree(ident("registeredRequests"), newLit(scalarReqKey)), - handlerProc, - ) - - var scalarParams = @[ - ident("cint"), - newIdentDefs(ident("ctx"), ctxType), - newIdentDefs(ident("callback"), ident("FFICallBack")), - newIdentDefs(ident("userData"), ident("pointer")), - ] - for i in 0 ..< extraParamNames.len: - scalarParams.add(newIdentDefs(ident(extraParamNames[i]), extraParamTypes[i])) - - let ffiBody = newStmtList() - ffiBody.add buildCtxGuard() - - let initScalarCall = newTree( - nnkCall, - newDotExpr(ident("FFIThreadRequest"), ident("initScalar")), - ident("callback"), - ident("userData"), - newDotExpr(newLit(scalarReqKey), ident("cstring")), - ) - for i in 0 ..< extraParamNames.len: - initScalarCall.add(newCall(ident("ffiPackScalar"), ident(extraParamNames[i]))) - + ## The scalar fast path lives in `ffi_scalar`; here we only build the shared + ## dispatch pieces (same helpers the usual path uses) and hand them over, so + ## the base macro carries none of the inline pack/unpack machinery. let reqPtrIdent = genSym(nskLet, "reqPtr") - ffiBody.add newLetStmt(reqPtrIdent, initScalarCall) - ffiBody.add buildSendAndReply(reqPtrIdent) - - let ffiProc = buildCExportProc(scalarParams, ffiBody) - - # Registered (not just skipped) so the compile-time metadata stays - # introspectable; `bindableProcs` drops it from foreign codegen. - var scalarMeta = procMeta - scalarMeta.scalarFastPath = true - ffiProcRegistry.add(scalarMeta) - - return newStmtList(helperProc, registerAssign, ffiProc) + buildScalarPath( + helperProc = buildAsyncHelperProc(), + ctxGuard = buildCtxGuard(), + reqPtrIdent = reqPtrIdent, + sendAndReply = buildSendAndReply(reqPtrIdent), + userProcName = userProcName, + cExportProcName = cExportProcName, + cExportName = cExportName, + ctxType = ctxType, + camelName = camelName, + extraParamNames = extraParamNames, + extraParamTypes = extraParamTypes, + procMeta = procMeta, + ) if abiFormat == ABIFormat.C and not scalarEligible: gateABIFormat(abiFormat, "`.ffi.` proc") diff --git a/ffi/internal/ffi_scalar.nim b/ffi/internal/ffi_scalar.nim new file mode 100644 index 0000000..a5cded5 --- /dev/null +++ b/ffi/internal/ffi_scalar.nim @@ -0,0 +1,172 @@ +## CBOR-free scalar fast path for all-scalar `{.ffi: "abi = c".}` methods. +## +## Kept out of the base `ffi` macro so the usual CBOR/async dispatch path in +## `ffi_macro.nim` stays simple: the macro only decides eligibility +## (`isScalarOnly`) and, when it applies, hands the whole codegen to +## `buildScalarPath`. +## +## A scalar proc's C export takes its scalar args directly (no +## `reqCbor`/`reqCborLen`), packs them inline into the request (no envelope +## `c_malloc`, no CBOR), and the FFI-thread handler unpacks them, runs the user +## body, and returns the result as raw bytes (`ffiScalarRetBytes`). + +import std/macros +import ../codegen/meta + +const scalarPodTypeNames = [ + "int", "int8", "int16", "int32", "int64", "uint", "uint8", "uint16", "uint32", + "uint64", "byte", "float", "float32", "float64", "bool", +] + ## Fixed-width POD scalars that fit a single `uint64` slot and survive the + ## async hop by value — the payload the scalar fast path inlines into the + ## request (no heap copy). `cstring`/`string` are intentionally absent as + ## *params*: they are pointers to caller memory the FFI thread reads later, + ## so they'd need a copy, defeating the zero-alloc promise. + +func isScalarParamTypeName*(name: string): bool = + ## A param type eligible for the CBOR-free scalar fast path. + name in scalarPodTypeNames + +func isScalarReturnTypeName*(name: string): bool = + ## A return type eligible for the scalar fast path. Unlike params, a + ## `string`/`cstring` return is fine: the handler produces the bytes and they + ## ride back raw (like the error path), so no caller memory is aliased. + name in scalarPodTypeNames or name == "string" or name == "cstring" + +func isScalarOnly*(p: FFIProcMeta): bool = + ## True iff `p` is a plain `{.ffi.}` method whose every wire param and return + ## is scalar — the whole signature crosses without CBOR or `_CWire`. Handles + ## and raw pointers are excluded (a handle needs a ctx-registry round-trip; + ## a pointer never crosses). Pure over the compile-time metadata. + if p.kind != FFIKind.FFI: + return false + if p.returnIsPtr or p.returnIsHandle: + return false + if not isScalarReturnTypeName(p.returnTypeName): + return false + for ep in p.extraParams: + if ep.isPtr or ep.isHandle or not isScalarParamTypeName(ep.typeName): + return false + true + +func bindableProcs*(procs: seq[FFIProcMeta]): seq[FFIProcMeta] = + ## The procs the foreign-binding generators emit for. Scalar-fast-path procs + ## are dropped: their C export takes inline scalar args, not the CBOR + ## `(reqCbor, reqCborLen)` shape the current codegen assumes, so emitting a + ## CBOR caller for them would be wrong. Foreign codegen is a follow-up. + var kept: seq[FFIProcMeta] = @[] + for p in procs: + if not p.scalarFastPath: + kept.add(p) + kept + +proc buildScalarPath*( + helperProc, ctxGuard, reqPtrIdent, sendAndReply: NimNode, + userProcName, cExportProcName: NimNode, + cExportName: string, + ctxType: NimNode, + camelName: string, + extraParamNames: seq[string], + extraParamTypes: seq[NimNode], + procMeta: FFIProcMeta, +): NimNode {.compileTime.} = + ## Emits the scalar-fast-path codegen for one `.ffi.` proc. The generic + ## dispatch pieces (`helperProc`, `ctxGuard`, `sendAndReply`) are built by the + ## caller from the same shared helpers the usual path uses, so this only owns + ## the scalar-specific inline pack / unpack / raw-bytes wiring. + let scalarReqKey = camelName & "Req" + + let reqIdent = genSym(nskLet, "ffiReq") + let ctxHandlerName = genSym(nskLet, "ffiCtxHandler") + let handlerBody = newStmtList() + handlerBody.add quote do: + let `reqIdent` = cast[ptr FFIThreadRequest](request) + let `ctxHandlerName` = cast[`ctxType`](reqHandler) + + let helperCall = newTree(nnkCall, userProcName) + let ctxMyLib = newDotExpr(newTree(nnkDerefExpr, ctxHandlerName), ident("myLib")) + helperCall.add(newTree(nnkDerefExpr, ctxMyLib)) + for i in 0 ..< extraParamNames.len: + let argIdent = ident(extraParamNames[i]) + let slot = nnkBracketExpr.newTree( + newDotExpr(newTree(nnkDerefExpr, reqIdent), ident("scalarArgs")), newLit(i) + ) + handlerBody.add( + newLetStmt(argIdent, newCall(ident("ffiUnpackScalar"), slot, extraParamTypes[i])) + ) + helperCall.add(argIdent) + + let retValIdent = genSym(nskLet, "retVal") + handlerBody.add quote do: + let `retValIdent` = (await `helperCall`).valueOr: + return err($error) + return ok(ffiScalarRetBytes(`retValIdent`)) + + let seqByteResult = nnkBracketExpr.newTree( + ident("Future"), + nnkBracketExpr.newTree( + ident("Result"), + nnkBracketExpr.newTree(ident("seq"), ident("byte")), + ident("string"), + ), + ) + let handlerProc = newProc( + name = newEmptyNode(), + params = @[ + seqByteResult, + newIdentDefs(ident("request"), ident("pointer")), + newIdentDefs(ident("reqHandler"), ident("pointer")), + ], + body = handlerBody, + pragmas = nnkPragma.newTree(ident("async")), + ) + let registerAssign = newAssignment( + nnkBracketExpr.newTree(ident("registeredRequests"), newLit(scalarReqKey)), + handlerProc, + ) + + var scalarParams = @[ + ident("cint"), + newIdentDefs(ident("ctx"), ctxType), + newIdentDefs(ident("callback"), ident("FFICallBack")), + newIdentDefs(ident("userData"), ident("pointer")), + ] + for i in 0 ..< extraParamNames.len: + scalarParams.add(newIdentDefs(ident(extraParamNames[i]), extraParamTypes[i])) + + let ffiBody = newStmtList() + ffiBody.add ctxGuard + + let initScalarCall = newTree( + nnkCall, + newDotExpr(ident("FFIThreadRequest"), ident("initScalar")), + ident("callback"), + ident("userData"), + newDotExpr(newLit(scalarReqKey), ident("cstring")), + ) + for i in 0 ..< extraParamNames.len: + initScalarCall.add(newCall(ident("ffiPackScalar"), ident(extraParamNames[i]))) + + ffiBody.add newLetStmt(reqPtrIdent, initScalarCall) + ffiBody.add sendAndReply + + let ffiProc = newProc( + name = postfix(cExportProcName, "*"), + params = scalarParams, + body = ffiBody, + pragmas = newTree( + nnkPragma, + ident("dynlib"), + newTree(nnkExprColonExpr, ident("exportc"), newStrLitNode(cExportName)), + ident("cdecl"), + newTree(nnkExprColonExpr, ident("raises"), newTree(nnkBracket)), + ), + ) + + # Registered (not just skipped) so the compile-time metadata stays + # introspectable; `bindableProcs` drops it from foreign codegen. + var scalarMeta = procMeta + scalarMeta.scalarFastPath = true + ffiProcRegistry.add(scalarMeta) + + newStmtList(helperProc, registerAssign, ffiProc) diff --git a/tests/unit/test_scalar_fastpath.nim b/tests/unit/test_scalar_fastpath.nim index 2421e9a..750054b 100644 --- a/tests/unit/test_scalar_fastpath.nim +++ b/tests/unit/test_scalar_fastpath.nim @@ -12,6 +12,7 @@ import unittest2 import results import ffi import ffi/codegen/meta +import ffi/internal/ffi_scalar type ScalarLib = object base: int