This repository was archived by the owner on Oct 12, 2022. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 411
Cleanup and fix implementation of _d_array{setlengthT,catnTX}Trace #2673
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| /** | ||
| This module contains utility functions to help the implementation of the runtime hook | ||
|
|
||
| Copyright: Copyright Digital Mars 2000 - 2019. | ||
| License: Distributed under the | ||
| $(LINK2 http://www.boost.org/LICENSE_1_0.txt, Boost Software License 1.0). | ||
| (See accompanying file LICENSE) | ||
| Source: $(DRUNTIMESRC rt/_array/_utils.d) | ||
| */ | ||
| module rt.array.utils; | ||
|
|
||
| import core.internal.traits : Parameters; | ||
|
|
||
| private auto gcStatsPure() nothrow pure | ||
| { | ||
| import core.memory : GC; | ||
|
|
||
| auto impureBypass = cast(GC.Stats function() pure nothrow)&GC.stats; | ||
| return impureBypass(); | ||
| } | ||
|
|
||
| private ulong accumulatePure(string file, int line, string funcname, string name, ulong size) nothrow pure | ||
| { | ||
| static ulong impureBypass(string file, int line, string funcname, string name, ulong size) @nogc nothrow | ||
| { | ||
| import core.internal.traits : externDFunc; | ||
|
|
||
| alias accumulate = externDFunc!("rt.profilegc.accumulate", void function(string file, uint line, string funcname, string type, ulong sz) @nogc nothrow); | ||
| accumulate(file, line, funcname, name, size); | ||
| return size; | ||
| } | ||
|
|
||
| auto func = cast(ulong function(string file, int line, string funcname, string name, ulong size) @nogc nothrow pure)&impureBypass; | ||
| return func(file, line, funcname, name, size); | ||
| } | ||
|
|
||
| /** | ||
| * TraceGC wrapper around runtime hook `Hook`. | ||
| * Params: | ||
| * T = Type of hook to report to accumulate | ||
| * Hook = The hook to wrap | ||
| * errorMessage = The error message incase `version != D_TypeInfo` | ||
| * file = File that called `HookTraceImpl` | ||
| * line = Line inside of `file` that called `HookTraceImpl` | ||
| * funcname = Function that called `HookTraceImpl` | ||
| * parameters = Parameters that will be used to call `Hook` | ||
| * Bugs: | ||
| * This function template was ported from a much older runtime hook that bypassed safety, | ||
| * purity, and throwabilty checks. To prevent breaking existing code, this function template | ||
| * is temporarily declared `@trusted pure nothrow` until the implementation can be brought up to modern D expectations. | ||
| */ | ||
| auto HookTraceImpl(T, alias Hook, string errorMessage)(string file, int line, string funcname, Parameters!Hook parameters) @trusted pure nothrow | ||
| { | ||
| version (D_TypeInfo) | ||
| { | ||
| pragma(inline, false); | ||
| string name = T.stringof; | ||
|
|
||
| // FIXME: use rt.tracegc.accumulator when it is accessable in the future. | ||
| version (tracegc) | ||
| { | ||
| import core.stdc.stdio; | ||
|
|
||
| printf("%sTrace file = '%.*s' line = %d function = '%.*s' type = %.*s\n", | ||
| Hook.stringof.ptr, | ||
| file.length, file.ptr, | ||
| line, | ||
| funcname.length, funcname.ptr, | ||
| name.length, name.ptr | ||
| ); | ||
| } | ||
|
|
||
| ulong currentlyAllocated = gcStatsPure().allocatedInCurrentThread; | ||
|
|
||
| scope(exit) | ||
| { | ||
| ulong size = gcStatsPure().allocatedInCurrentThread - currentlyAllocated; | ||
| if (size > 0) | ||
| if (!accumulatePure(file, line, funcname, name, size)) { | ||
| // This 'if' and 'assert' is needed to force the compiler to not remove the call to | ||
| // `accumulatePure`. It really want to do that while optimizing as the function is | ||
| // `pure` and it does not influence the result of this hook. | ||
|
|
||
| // `accumulatePure` returns the value of `size`, which can never be zero due to the | ||
| // previous 'if'. So this assert will never be triggered. | ||
| assert(0); | ||
| } | ||
| } | ||
| return Hook(parameters); | ||
| } | ||
| else | ||
| assert(0, errorMessage); | ||
| } | ||
|
|
||
This file contains hidden or 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, reading this I think there's may be a deeper issue whenever we have 'fake' pure code which internally calls impure code.
I guess the intent of such code is to not influence purity of the caller (i.e. completely optimizing away the caller would be fine), but the callee is not really pure, so it should not be removed on it's own. I wonder whether this can be expressed in any theoretically sound way, @tgehr any insights here?
It seems to me that the problem is that we lie to the compiler and simply tell it
this function is pure. Maybe we should rather tell itIgnore purity checks for this call, but the function is not pure. Intuitively I'd say aShould work: if bar is not inlined, this would tell the compiler to keep the call, but ignore purity for purity calculation of the callee. With inlining, the compiler should be able to see the side effects anyway and should not optimize these away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see how your
assertcode has the same effect though: It marks the return value as used, as long as the call is not inlined. When it's inlined, the compiler may remove the assert, but it will see the impure parts of accumulatePure and keep these. This can probably also somehow solved like this: https://stackoverflow.com/questions/40122141/preventing-compiler-optimizations-while-benchmarking, but I'd prefer a compiler hook which should also work for void functions.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been thinking about introducing
HookCallExpor a bool inCallExpthat ignore the purity and somehow allowed the code to be inline, which is disallowed right nowpragma(inline, false), whilst being able to be detected as the hook (HookCompoundStatement?) so the CTFE intercept code works.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like a great idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jpf91 This was what
__mutablefunctions were for in the original__mutableproposal. Inlining for such functions is not so easy if you want to preserve constraints on evaluation order precisely.