Adding json output (feature request #14711)
Changes PlannedPublic

Authored by jproyo on Mar 19 2018, 9:06 PM.

Details

Reviewers
bgamari
jproyo created this revision.Mar 19 2018, 9:06 PM
jproyo planned changes to this revision.Mar 20 2018, 9:17 AM

Hi Community,

I have been testing this and it seems there is an issue building current master HEAD rather than with this commit i performed.

I've been wondering around to see what has happened and it seems it is relate with another commit in master whose sha is 2d4bda2e4a.

I have tested it cloning a new GHC's repository copy from scratch without adding anything and it is not compiling.

This is the output cloning GHC as it is right now in master HEAD after running the following 3 initial build commands.

./boot
./configure
make -j4

This is the output after running make -j4

rts/Stats.c:683:17: error:
     warning: format specifies type 'size_t' (aka 'unsigned long') but the argument has type 'unsigned long long' [-Wformat]
                    stats.max_live_bytes  / (1024 * 1024),
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |
683 |                 stats.max_live_bytes  / (1024 * 1024),
    |                 ^

rts/Stats.c:684:17: error:
     warning: format specifies type 'size_t' (aka 'unsigned long') but the argument has type 'unsigned long long' [-Wformat]
                    sum->fragmentation_bytes / (1024 * 1024));
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |
684 |                 sum->fragmentation_bytes / (1024 * 1024));
    |                 ^

rts/Stats.c:741:39: error:
     error: no member named 'rc_cpu_ns' in 'struct RTSSummaryStats_'
                    TimeToSecondsDbl(sum->rc_cpu_ns),
                                     ~~~  ^
    |
741 |                 TimeToSecondsDbl(sum->rc_cpu_ns),
    |                                       ^

rts/Stats.c:29:39: error:
     note: expanded from macro 'TimeToSecondsDbl'
   |
29 | #define TimeToSecondsDbl(t) ((double)(t) / TIME_RESOLUTION)
   |                                       ^
#define TimeToSecondsDbl(t) ((double)(t) / TIME_RESOLUTION)
                                      ^

rts/Stats.c:742:39: error:
     error: no member named 'rc_elapsed_ns' in 'struct RTSSummaryStats_'
                    TimeToSecondsDbl(sum->rc_elapsed_ns));
                                     ~~~  ^
    |
742 |                 TimeToSecondsDbl(sum->rc_elapsed_ns));
    |                                       ^

rts/Stats.c:29:39: error:
     note: expanded from macro 'TimeToSecondsDbl'
   |
29 | #define TimeToSecondsDbl(t) ((double)(t) / TIME_RESOLUTION)
   |                                       ^
#define TimeToSecondsDbl(t) ((double)(t) / TIME_RESOLUTION)
                                      ^

rts/Stats.c:885:7: error:
     warning: format specifies type 'size_t' (aka 'unsigned long') but the argument has type 'unsigned long long' [-Wformat]
          stats.max_mem_in_use_bytes / (1024 * 1024));
    |
885 |       stats.max_mem_in_use_bytes / (1024 * 1024));
    |       ^
~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

rts/Stats.c:867:62: error:
     note: expanded from macro 'MR_STAT'
        statsPrintf(" ,(\"" field_name "\", \"%" format "\")\n", value)
                                              ~~~                ^~~~~
    |
867 |     statsPrintf(" ,(\"" field_name "\", \"%" format "\")\n", value)
    |                                                              ^

rts/Stats.c:904:58: error:
     error: no member named 'hp_cpu_ns' in 'struct RTSSummaryStats_'
        MR_STAT("hc_cpu_seconds", "f", TimeToSecondsDbl(sum->hp_cpu_ns));
                                                        ~~~  ^
    |
904 |     MR_STAT("hc_cpu_seconds", "f", TimeToSecondsDbl(sum->hp_cpu_ns));
    |                                                          ^

rts/Stats.c:29:39: error:
     note: expanded from macro 'TimeToSecondsDbl'
   |
29 | #define TimeToSecondsDbl(t) ((double)(t) / TIME_RESOLUTION)
   |                                       ^
#define TimeToSecondsDbl(t) ((double)(t) / TIME_RESOLUTION)
                                      ^

rts/Stats.c:867:62: error:
     note: expanded from macro 'MR_STAT'
        statsPrintf(" ,(\"" field_name "\", \"%" format "\")\n", value)
                                                                 ^~~~~
    |
867 |     statsPrintf(" ,(\"" field_name "\", \"%" format "\")\n", value)
    |                                                              ^

rts/Stats.c:905:59: error:
     error: no member named 'hp_elapsed_ns' in 'struct RTSSummaryStats_'
        MR_STAT("hc_wall_seconds", "f", TimeToSecondsDbl(sum->hp_elapsed_ns));
                                                         ~~~  ^
    |
905 |     MR_STAT("hc_wall_seconds", "f", TimeToSecondsDbl(sum->hp_elapsed_ns));
    |                                                           ^

rts/Stats.c:29:39: error:
     note: expanded from macro 'TimeToSecondsDbl'
   |
29 | #define TimeToSecondsDbl(t) ((double)(t) / TIME_RESOLUTION)
   |                                       ^
#define TimeToSecondsDbl(t) ((double)(t) / TIME_RESOLUTION)
                                      ^

rts/Stats.c:867:62: error:
     note: expanded from macro 'MR_STAT'
        statsPrintf(" ,(\"" field_name "\", \"%" format "\")\n", value)
                                                                 ^~~~~
    |
867 |     statsPrintf(" ,(\"" field_name "\", \"%" format "\")\n", value)
    |                                                              ^

rts/Stats.c:940:47: error:
     warning: format specifies type 'size_t' (aka 'unsigned long') but the argument has type 'uint64_t' (aka 'unsigned long long') [-Wformat]
        MR_STAT("fragmentation_bytes", FMT_SizeT, sum->fragmentation_bytes);
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
    |
940 |     MR_STAT("fragmentation_bytes", FMT_SizeT, sum->fragmentation_bytes);
    |                                               ^

rts/Stats.c:867:62: error:
     note: expanded from macro 'MR_STAT'
        statsPrintf(" ,(\"" field_name "\", \"%" format "\")\n", value)
                                              ~~~                ^~~~~
    |
867 |     statsPrintf(" ,(\"" field_name "\", \"%" format "\")\n", value)
    |                                                              ^
4 warnings and 4 errors generated.
`gcc' failed in phase `C Compiler'. (Exit code: 1)
make[1]: *** [rts/dist/build/Stats.p_o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [all] Error 2

How can i commit this again to move forward with the revision?

Thanks,

bgamari added a comment.EditedMar 20 2018, 11:18 AM

Hi Community,

I have been testing this and it seems there is an issue building current master HEAD rather than with this commit i performed.

I have reverted the failing commit. If you rebase the tree should build.

Ben fixed this in D4516 which should be merged shortly and fix the build.

Cabal has similar needs for a minimal json output library: https://github.com/haskell/cabal/blob/master/cabal-install/Distribution/Client/Utils/Json.hs. You could use that, it will make sure the string escaping is correct; might save us a bit of trouble in the long run.

jproyo added a comment.EditedMar 23 2018, 8:06 AM

Cabal has similar needs for a minimal json output library: https://github.com/haskell/cabal/blob/master/cabal-install/Distribution/Client/Utils/Json.hs. You could use that, it will make sure the string escaping is correct; might save us a bit of trouble in the long run.

Great to know. Since i noticed xml current report was built by hand i thought i couldnt use any external library.

I am going to change in that sense.

Best,

Great to know. Since i noticed xml current report was built by hand i thought i couldnt use any external library.

Well, I am not suggesting a dependency to an external library but copying that module from Cabal source :)

There is already a small JSON library embedded in GHC.

Great to know. Since i noticed xml current report was built by hand i thought i couldnt use any external library.

Well, I am not suggesting a dependency to an external library but copying that module from Cabal source :)

There is already a JSON module in the GHC tree (which I added). But I think it is ok to use modules from Cabal as GHC depends on it anyway?

There is already a small JSON library embedded in GHC.

Great to know. Since i noticed xml current report was built by hand i thought i couldnt use any external library.

Well, I am not suggesting a dependency to an external library but copying that module from Cabal source :)

There is already a JSON module in the GHC tree (which I added). But I think it is ok to use modules from Cabal as GHC depends on it anyway?

Great!! I will explore both alternatives proposals @aleksigalew and yours.

Let you know with a new revision request.

Thanks for the advice and support.

Best,

There is already a small JSON library embedded in GHC.

Great to know. Since i noticed xml current report was built by hand i thought i couldnt use any external library.

Well, I am not suggesting a dependency to an external library but copying that module from Cabal source :)

There is already a JSON module in the GHC tree (which I added). But I think it is ok to use modules from Cabal as GHC depends on it anyway?

Great!! I will explore both alternatives proposals @aleksigalew and yours.

Let you know with a new revision request.

Thanks for the advice and support.

Best,

I have explored Json's module inside compiler/utils and unfortunately it is not exported and in consequence cannot be consumed outside that module. Also i've tried to check if it is ok enough to copy inside hpc and compared with Cabal's one purposed by @alexbiehl i think i am going to move forward with Cabal's Json. It is shorter and also support Double, Float and other types, where compiler/utils doesn't.

Any thoughts or recommendations?

BTW, I have just realised i missed type in the other comment and made a reference to @aleksigalew. Sorry for that @aleksigalew!

Best,