Ensure we reset OutputtedFilenameList in profiling when a workspace is restored #1164
Conversation
src/profile.c
Outdated
| { | ||
| /* When we restore a workspace, we start a new profile. This is the | ||
| * only part of the profile which is stored in the GAP memory space, | ||
| * so we want to clear it |
There was a problem hiding this comment.
We "want" to clear it? Not "need" to clear it?
There was a problem hiding this comment.
I'm happy to change to 'need'
src/profile.c
Outdated
| @@ -1005,6 +1005,15 @@ static Int InitKernel ( | |||
| return 0; | |||
| } | |||
|
|
|||
| static Int PostRestore ( StructInitInfo * module ) | |||
| { | |||
| /* When we restore a workspace, we start a new profile. This is the | |||
There was a problem hiding this comment.
I was mildly confused when reading "This" here, until I realized that it referred to the text after the comment. Perhaps rephrase this as "The OutputtedFilenameList is ..." or "The list of outputted filenames is" ?
ChrisJefferson
force-pushed
the
profile-restore
branch
from
3bb7910
to
e60761a
Compare
Codecov Report
@@ Coverage Diff @@
## master #1164 +/- ##
==========================================
+ Coverage 66.8% 68.35% +1.55%
==========================================
Files 433 435 +2
Lines 228624 230662 +2038
==========================================
+ Hits 152725 157680 +4955
+ Misses 75899 72982 -2917
Continue to review full report at Codecov.
|
|
I have no idea why it looks like this is trashing coverage so badly. |
ChrisJefferson
force-pushed
the
profile-restore
branch
from
e60761a
to
ddd4921
Compare
ChrisJefferson
force-pushed
the
profile-restore
branch
from
ddd4921
to
0c9d1ff
Compare
This patch ensures that if we load a saved workspace, we correctly output filenames. This is only required when restoring a workspace where coverage/profile was active, with
--profor--cover.The workspace restores the variable
OutputtedFilenameList, which represents files we have already outputted. We want to clear that file, so our new profile includes the filenames.This is a pain to add a test for, but easy to eyeball -- looking at profiles generated they were previously missing filenames, now they aren't.