Skip to content

Commit

Permalink
internal: separate VM profiler data from ConfigRef (#899)
Browse files Browse the repository at this point in the history
## Summary

Store the data collected by the VM profiler with the profiler itself,
instead of storing it with the global configuration state (`ConfigRef`).

This allows for moving the profiler-related data types closer to the
`vmprofiler` module, and it also shrinks the surface of `ConfigRef` and
the `options` module.

## Details

* move all profiler-related data types to `vmdef`; they should ideally
  be placed in the `vmprofiler` module, but that would currently
  introduce cyclic dependencies
* `vmprofiler.dump` now accepts a `Profiler` instance; for rendering the
  data associated with a VM context, the new
  `compilerbridge.dumpVmProfilerData` procedure is now used
  • Loading branch information
zerbina authored Sep 14, 2023
1 parent e3e6ef3 commit ce91535
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 30 deletions.
6 changes: 0 additions & 6 deletions compiler/ast/lineinfos.nim
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ type
col*: int16
fileIndex*: FileIndex

SourceLinePosition* = tuple
## Identifies a line in a source file. Only intended for use by
## the profiler.
fileIndex: typeof(TLineInfo.fileIndex)
line: typeof(TLineInfo.line)

LineColPair* = tuple
line: typeof(TLineInfo.line)
col: typeof(TLineInfo.col)
Expand Down
3 changes: 1 addition & 2 deletions compiler/front/main.nim
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ import
compiler/vm/[
compilerbridge, # Configuration file evaluation, `nim e`
vmbackend, # VM code generation
vmprofiler
]

import compiler/ic/[
Expand Down Expand Up @@ -686,7 +685,7 @@ proc mainCommand*(graph: ModuleGraph) =
srcCodeOrigin: instLoc()))

if optProfileVM in conf.globalOptions:
conf.writeln cmdOutUserProf, conf.dump(conf.vmProfileData)
conf.writeln cmdOutUserProf, dumpVmProfilerData(graph)

if conf.errorCounter == 0 and conf.cmd notin {cmdTcc, cmdDump, cmdNop}:
if conf.isEnabled(rintSuccessX):
Expand Down
12 changes: 0 additions & 12 deletions compiler/front/options.nim
Original file line number Diff line number Diff line change
Expand Up @@ -161,13 +161,6 @@ type

Suggestions* = seq[Suggest]

ProfileInfo* = object
time*: float
count*: int

ProfileData* = ref object
data*: TableRef[SourceLinePosition, ProfileInfo]

StdOrrKind* = enum
stdOrrStdout
stdOrrStderr
Expand Down Expand Up @@ -303,7 +296,6 @@ type
writeHook*: proc(conf: ConfigRef, output: string, flags: MsgFlags) {.closure.}
structuredReportHook*: ReportHook
astDiagToLegacyReport*: proc(conf: ConfigRef, d: PAstDiag): Report
vmProfileData*: ProfileData
setMsgFormat*: proc(config: ConfigRef, fmt: MsgFormatKind) {.closure.}
## callback that sets the message format for legacy reporting, needs to
## set before CLI handling, because reports are just that awful
Expand Down Expand Up @@ -772,9 +764,6 @@ template newPackageCache*(): untyped =
else:
modeCaseSensitive)

proc newProfileData(): ProfileData =
ProfileData(data: newTable[SourceLinePosition, ProfileInfo]())

proc isDefined*(conf: ConfigRef; symbol: string): bool

const defaultHackController = HackController(
Expand Down Expand Up @@ -933,7 +922,6 @@ proc newConfigRef*(hook: ReportHook): ConfigRef =
),
suggestMaxResults: 10_000,
maxLoopIterationsVM: 10_000_000,
vmProfileData: newProfileData(),
spellSuggestMax: spellSuggestSecretSauce,
)
initConfigRefCommon(result)
Expand Down
9 changes: 9 additions & 0 deletions compiler/vm/compilerbridge.nim
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import
vmjit,
vmlegacy,
vmops,
vmprofiler,
vmtypegen,
vmutils,
vm
Expand Down Expand Up @@ -655,6 +656,14 @@ proc evalMacroCall*(module: PSym; idgen: IdGenerator; g: ModuleGraph;

result = evalMacroCall(c.jit, c.vm, call, args, sym)

proc dumpVmProfilerData*(graph: ModuleGraph): string =
## Dumps the profiler data collected by the profiler of the VM instance
## associated with `graph` to a string.
let c = PEvalContext(graph.vm)
result =
if c != nil: dump(graph.config, c.vm.profiler)
else: ""

# ----------- the VM-related compilerapi -----------

# NOTE: it might make sense to move the VM-related compilerapi into
Expand Down
16 changes: 16 additions & 0 deletions compiler/vm/vmdef.nim
Original file line number Diff line number Diff line change
Expand Up @@ -734,10 +734,25 @@ type
## instruction during cleanup. -1 indicates that
## no clean-up is happening

ProfileInfo* = object
## Profiler data for a single procedure.
time*: float ## the time spent on executing instructions (exclusive)
count*: int ## the number of instruction executed (exclusive)

SourceLinePosition* = tuple
## Identifies a line in a source file. Only intended for use by
## the profiler.
fileIndex: typeof(TLineInfo.fileIndex)
line: typeof(TLineInfo.line)

Profiler* = object
# XXX: move this type to the ``vmprofiler`` module once possible
tEnter*: float
sframe*: StackFrameIndex ## The current stack frame

data*: TableRef[SourceLinePosition, ProfileInfo]
# XXX: use a ``Table`` instead of a ``TableRef``

func `<`*(a, b: FieldIndex): bool {.borrow.}
func `<=`*(a, b: FieldIndex): bool {.borrow.}
func `==`*(a, b: FieldIndex): bool {.borrow.}
Expand Down Expand Up @@ -866,6 +881,7 @@ proc initCtx*(module: PSym; cache: IdentCache; g: ModuleGraph;
loopIterations: g.config.maxLoopIterationsVM,
comesFromHeuristic: unknownLineInfo,
callbacks: @[],
profiler: Profiler(data: newTable[SourceLinePosition, ProfileInfo]()),
cache: cache,
config: g.config,
graph: g,
Expand Down
19 changes: 9 additions & 10 deletions compiler/vm/vmprofiler.nim
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,26 @@ proc enter*(prof: var Profiler, c: TCtx, sframe: StackFrameIndex) {.inline.} =
proc leaveImpl(prof: var Profiler, c: TCtx) {.noinline.} =
let tLeave = cpuTime()
var frameIdx = prof.sframe
var data = c.config.vmProfileData.data
while frameIdx >= 0:
let frame = c.sframes[frameIdx]
if frame.prc != nil:
let li = (frame.prc.info.fileIndex, frame.prc.info.line)
if li notin data:
data[li] = ProfileInfo()
data[li].time += tLeave - prof.tEnter
if li notin prof.data:
prof.data[li] = ProfileInfo()
prof.data[li].time += tLeave - prof.tEnter
if frameIdx == prof.sframe:
inc data[li].count
inc prof.data[li].count
dec frameIdx

proc leave*(prof: var Profiler, c: TCtx) {.inline.} =
if optProfileVM in c.config.globalOptions:
leaveImpl(prof, c)

proc dump*(conf: ConfigRef, pd: ProfileData): string =
## constructs a string containing a report of vm exectuion based on the given
## `ProfileData`. The report is formatted and ready to print to console or
## similar interface.
var data = pd.data
proc dump*(conf: ConfigRef, prof: Profiler): string =
## Constructs a string containing a report of VM execution based on the
## data collected by `prof`. The report is formatted and ready to print to
## console or similar interface.
var data = prof.data
result = "\nprof: µs #instr location\n"
for i in 0..<32:
var infoMax: ProfileInfo
Expand Down

0 comments on commit ce91535

Please sign in to comment.