Skip to content

Commit

Permalink
internal: speed up FileInfoIdx-from-path lookup (#859)
Browse files Browse the repository at this point in the history
## Summary

Cache the result of file-info-from-absolute-path lookups, avoiding
unnecessary filesystem IO incurred by expanding file paths. This
significantly increases the speed of the incremental compilation mode
(which performs a lot of those lookups), with compile time reductions
of up to 90% (measured with the `tstdlib_import_changed.nim` test).

## Details

Add the `rawPathToIndexTbl` table to `MsgFormat`. It associates the
unprocessed absolute file paths passed to `fileInfoIdx` with their
resolved-to `FileInfoIdx`, preventing the repeated expansion of the
same file paths.

In addition, the line overrides in the `assert` and `jsAssert`
templates are changed to use the no-argument form (`{.line.}`). The
latter is semantically equivalent to the previous `{.line: loc.}`,
while not performing an unnecessary `FileInfoIdx` -> filename ->
`FileInfoIdx` round-trip.

---------

Co-authored-by: zerbina <100542850+zerbina@users.noreply.github.com>
  • Loading branch information
bung87 and zerbina authored Aug 27, 2023
1 parent 9efc16f commit 5737047
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 2 deletions.
8 changes: 8 additions & 0 deletions compiler/front/options.nim
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ type
writtenSemReports*: ReportSet
lastError*: TLineInfo
filenameToIndexTbl*: Table[string, FileIndex]
rawPathToIndexTbl*: Table[string, FileIndex] ## maps non-canonicalized
## paths of known-files to the corresponding file index
fileInfos*: seq[TFileInfo] ## Information about all known source files
## is stored in this field - full/relative paths, list of line etc.
## (For full list see `TFileInfo`)
Expand All @@ -112,6 +114,7 @@ proc initMsgConfig*(): MsgConfig =
result.fileInfos = @[]
result.errorOutputs = {eStdOut, eStdErr}
result.filenameToIndexTbl["???"] = FileIndex(-1)
result.rawPathToIndexTbl = initTable[string, FileIndex]()

func incl*(s: var ReportSet, id: NodeId) = s.ids.incl uint32(id)
func contains*(s: var ReportSet, id: NodeId): bool = s.ids.contains uint32(id)
Expand Down Expand Up @@ -1172,6 +1175,9 @@ proc fileInfoKnown*(conf: ConfigRef; filename: AbsoluteFile): bool =
result = conf.m.filenameToIndexTbl.hasKey(canon.string)

proc fileInfoIdx*(conf: ConfigRef; filename: AbsoluteFile; isKnownFile: var bool): FileIndex =
result = conf.m.rawPathToIndexTbl.getOrDefault(filename.string, InvalidFileIdx)
if result != InvalidFileIdx:
return
var
canon: AbsoluteFile
pseudoPath = false
Expand All @@ -1198,6 +1204,8 @@ proc fileInfoIdx*(conf: ConfigRef; filename: AbsoluteFile; isKnownFile: var bool
else: relativeTo(canon, conf.projectPath)))
conf.m.filenameToIndexTbl[canon2] = result

conf.m.rawPathToIndexTbl[filename.string] = result

proc fileInfoIdx*(conf: ConfigRef; filename: AbsoluteFile): FileIndex =
var dummy: bool
result = fileInfoIdx(conf, filename, dummy)
Expand Down
2 changes: 1 addition & 1 deletion lib/js/jsconsole.nim
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ since (1, 5):
const
loc = instantiationInfo(fullPaths = compileOption("excessiveStackTrace"))
msg = getMsg(loc, astToStr(assertion)).cstring
{.line: loc.}:
{.line.}:
{.emit: ["console.assert(", assertion, ", ", msg, ");"].}

func dir*(console: Console; obj: auto) {.importjs.}
Expand Down
2 changes: 1 addition & 1 deletion lib/system/assertions.nim
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ template assertImpl(cond: bool, msg: string, expr: string, enabled: static[bool]
ploc = $loc
bind instantiationInfo
mixin failedAssertImpl
{.line: loc.}:
{.line.}:
if not cond:
failedAssertImpl(ploc & " `" & expr & "` " & msg)

Expand Down

0 comments on commit 5737047

Please sign in to comment.