Initial version of the hot-code reloading support for native targets #10729
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tream Nim after commit 82c009a would be impossible.
- implemented the HCR test - almost works... - fix the issue on Unix where for executable targets the source file for the main module of a project in nimcache was being overwritten with the binary itself (and thus the actual source code was lost) - fixing embedded paths to shared objects on unix (the "lib" prefix was being prepended to the entire path instead of just the filename) - other fixes - removing unnecessary includes since that file is already included in chcks.nim which is in turn included in system.nim (and previously was getting imported in chcks.nim but then system.nim improts something... and that breaks HCR (perhaps it could be fixed but it would be nice not to import anything in system))
more stable mangling of parameter names when HCR is on the length of the static arrays in the DatInit functions is now part of the name of the variables, so when they get resized they get also recreated more stable mangling for inline functions - no longer depends on the module which first used them work on the new complicated HCR test - turned surprisingly complex - WIP test now successfully passes even when re-running `koch test` (previously when the nimcache wasn't cold that lead to errors) better documentation calling setStackBottomWith for PreMain passes over the HcrInit/DatInit/Init calls of all modules are now in the proper order (first all of one type, then all of the next). Also typeinfo globals are registered (created) in a single pass before the DatInit pass (because of the way generic instantiations are handled) Fix the test suite execution on macOs fix for being able to query the program arguments when using HCR on posix! other fixes
is converted to a raw pointer
* Added {.executeOnReload.} pragma that indicates top-level statements
that should be executed on each reload. To make this work, I've modified
the way the `if (hcr_init_) {...}` guards are produced in the init code.
This still needs more work as the new guards seem to be inserted within
the previously generated guards.
This change also removes the need for `lastRegistedGlobal` in nimhcr.
* Implemented the `signatureHash` magic and the `hasModuleChanged` API
depending on it (the actual logic is not imlemented yet).
* Add the "hcr" prefix to all HCR-related symbols in the system module.
Added a new `hotcodereloading` module exporting the high-level API to
the user.
Besides being more hygienic, this was also required in order to make
it possible to use macros in the high-level API. Without the split,
`system` would have to import `macros`, which was going to produce
the well-known init problems.
* Attempted to solve the "GC markers problem".
Crashes were expected with the previous code, because the GC markers
were compiled as normal procs are registered in the GC. When their
module is unloaded, dangling pointers will remain in the GC tables.
To solve this issue, I don't register any GC markers when HCR is on,
but I add them to the HCR globals metadata and I use a single marker
registed in nimhcr during the initialization of the system module that
will be responsible for marking all globals.
- tuples can be returned and broken into different vars in global scope - added comments for the closnig scopes of the if statements in the init proc - the new executeOnReload pragma works now! - other fixes
…out of line (out of the class body) - we no longer need to forward-declare popCurrentExceptionEx
This is a temporary hack that breaks some tests. I'll investigate later how these can be fixed.
…lly a new line (or something like that) - these labels are intended only for GOTO labels anyway...
…ing debugged and hot code reloading is used at the same time
…the global pragma, also simplified code around loops in global scope which have local vars (actually globals)
…ions - they were causing trouble with hot code reloading when no semantic change propagates to the main module but a line directive got changed and thus the main module had to be recompiled since the .c code had changed
…removing only 1 copy of all the duplicates (after a few reloads)
After reviewing the code more carefully, I've noticed that the old logic will be broken when the user overrides the '--out:f' compiler option. Besides fixing this issues, I took the opportunity to implement the missing '--outdir:d' option. Other changes: * ./koch test won't overwrite any HCR and RTL builds located in nim/lib * HCR and RTL are compiled with --threads:on by default
Also fixes some typos and the expected output of the HCR integration test
…eaks HCR because a 2nd function pointer is emitted for this externed/rtl func
planetis-m
reviewed
Feb 24, 2019
| @@ -33,6 +33,7 @@ type | |||
| modules*: seq[PSym] ## indexed by int32 fileIdx | |||
| packageSyms*: TStrTable | |||
| deps*: IntSet # the dependency graph or potentially its transitive closure. | |||
| importDeps*: Table[FileIndex, seq[FileIndex]] # explicit import module dependencies | |||
| suggestMode*: bool # whether we are in nimsuggest mode or not. | |||
| invalidTransitiveClosure: bool | |||
| inclToMod*: Table[FileIndex, FileIndex] # mapping of include file to the | |||
There was a problem hiding this comment.
Use a Table[FileIndex, FileIndex] and the add proc
Araq
reviewed
Feb 25, 2019
| ## will be able to grow up to the size of the initially reserved | ||
| ## portion of the address space. | ||
|
|
||
| from ospaths import raiseOSError, osLastError |
Araq
reviewed
Feb 25, 2019
| @@ -8,7 +8,7 @@ | |||
| # | |||
|
|
|||
| # Implementation of some runtime checks. | |||
| import system/indexerrors | |||
| include system/indexerrors | |||
There was a problem hiding this comment.
The reasoning was for system to not import anything (chcks.nim is itself included in system). This is mostly because of the order of initialization of modules - up until this got introduced system didn't import anything. The difference is that if it is included like this, the contents of indexerrors will also be exported from system...
I see that recently system started importing and immediately after that exporting 2 modules - widestrs and io, but I didn't change them to being included. I might revisit this whole situation.
There was a problem hiding this comment.
System.nim must be able to import modules, we will depend more heavily on this feature in the future, but they can all be marked as .noReload or whatever you called it. ;-)
|
Merging it now that is still merges successfully. Will clean it up a bit later on. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.