Allow configuration of cycle detection via command line parameters.#1668
Allow configuration of cycle detection via command line parameters.#1668MartinNowak merged 5 commits intodlang:stablefrom
Conversation
|
Added tests for new switches. Working on changelog... |
|
Hm... changelog is already cleared. @MartinNowak, can you set up the changelog properly for this? I'll work on updating the docs. |
schveiguy
changed the title
|
OK, I can set up changelog when I add docs. So I can do all this. This PR is probably OK to merge, I just need to do docs/changelog. |
|
Changelog and docs done. |
- directly compare pointers instead of module indices - rename e (edge) -> imp (import)
src/rt/minfo.d
Outdated
| // print the message | ||
| buildCycleMessage(idx, midx, (string x) { | ||
| import core.stdc.stdio : fprintf, stderr; | ||
| fprintf(stderr, "%.*s", x.length, x.ptr); |
|
|
||
| // build the edges between each module. We may need this for printing, | ||
| // and also allows avoiding keeping a hash around for module lookups. | ||
| int[][] edges = (cast(int[]*)malloc((int[]).sizeof * _modules.length))[0 .. _modules.length]; |
There was a problem hiding this comment.
Is that really such a good idea?
Space for all edges could be quite big, right?
And we almost never need to print cycles.
There was a problem hiding this comment.
Will test against phobos and a vibe-d project.
There was a problem hiding this comment.
11KB phobos, 7KB dub-registry (uses pretty much all of vibe.d).
Time for sorting phobos went from 1000µs -> 500µs.
There was a problem hiding this comment.
The edges are all integers, and I'm only allocating enough for unique edges (I realloc to shrink after processing each module), and it's only for direct imports. If we have 1000 modules and conservatively estimate that each one imports 15 other modules, that's only 15,000 * 4 = 60kb.
Time for sorting phobos went from 1000µs -> 500µs
Nice!
src/rt/minfo.d
Outdated
| // preallocate enough space to store all the indexes | ||
| int *edge = cast(int*)malloc(int.sizeof * _modules.length); | ||
| size_t nEdges = 0; | ||
| foreach (e; m.importedModules) |
There was a problem hiding this comment.
I think imp would be easier to understand.
src/rt/minfo.d
Outdated
| { | ||
| if(auto impidx = e in modIndexes) | ||
| { | ||
| if (*impidx != i) |
There was a problem hiding this comment.
You could continue before the hash lookup.
if (e is m) continue; // self-import
| $(ROOT)/cycle_abort.done: RETCODE=1 | ||
| $(ROOT)/cycle_abort.done: LINES=5 | ||
| $(ROOT)/cycle_print.done: RETCODE=0 | ||
| $(ROOT)/cycle_print.done: LINES=8 |
- %.*s expects an int length
Yes, shouldn't have done that. Need to improve the changed tool to automatically gather the project changelog files. |
|
Auto-merge toggled on |
| buildCycleMessage(idx, midx, (string x) { | ||
| import core.stdc.stdio : fprintf, stderr; | ||
| fprintf(stderr, "%.*s", x.length, x.ptr); | ||
| fprintf(stderr, "%.*s", cast(int) x.length, x.ptr); |
2 participants
Changes in this:
NOTE: I have NOT tested this properly, because I cannot build dmd on MacOS, see https://issues.dlang.org/show_bug.cgi?id=16536. I'd like to add tests for the switches, but I'm not sure how to do this, especially since I cannot build. I'm throwing this in the mix to at least test the default functionality.I will add a changelog once this is more solid.
Update: Was able to download binary of MacOS dmd and run unit tests.