[m-rev.] for review: reporting stats NOT to stderr

[Julien Fischer]

Hi Zoltan,

On Mon, 8 Mar 2021, Zoltan Somogyi wrote:

> On Mon, 8 Mar 2021 14:52:25 +1100 (AEDT), Julien Fischer <jfischer at opturion.com> wrote:
>>> This diff makes the change only for the C backend. Would anyone
>>> volunteer to make the change to for the C# and Java backends?
>>
>> I will take a look at them.
>
> Thanks for that, but you should wait until we resolve the issue below.
>
>>> As long as the output is done by C code (as opposed to the C code
>>> constructing a big string containing all the stuff to be output),
>>> directing the output to a user-supplied io.text_output_stream requires
>>> converting that text_output_stream to a value of the C "FILE *" type.
>>> This requires access to the actual definition of the io.text_output_stream
>>> type, which is private to io.m. We could export it to benchmarking.m,
>>> but that is undesirable.
>>
>> MR_file(*MR_unwrap_output_stream(Stream)) should do it.
>> (That's what we currently do in a similar situation in browser/listing.m.)
>
> I didn't know about that. This macro makes it feasible to keep the foreign
> language code for report_*stats in benchmarking.m.

There is a downside to that macro; it's heavily dependent on exactly how
Mercury file streams are represented.  I think it's justifiable in this
case since:

1. The definition of the io stream types and how they are represented
are both under the control of the Mercury developers.

2. Mercury programs interacting with C code *do* sometimes require a
mechanism for getting at the underlying "FILE *" type associated with a
Mercury file stream; your modified version of the benchmarking code is
one such example; in G12's CPLEX binding we use the above to pass the
underlying file pointer from G12's logging stream to CPLEX, so that
the CPLEX logging messages end up in the G12 log stream.

> I will definitely add a comment to the private definition of the input_stream
> and output_stream types about these macros.
>
>> That said, I don't have any objections to do it the way you've done here
>> either.
>
> The current situation, where we have report_*stats defined in both
> benchmarking.m (in impure form) and in io.m (in pure form) is *clearly*
> not ideal. While report_*stats is not an *exact* fit for benchmarking.m,
> since the info it provides can be used for purposes other than benchmarking
> (especially when printing tabling stats), the fit is clearly better in benchmarking.m
> than in io.m, so if we want to confine the report_*stats predicates to just
> one library module, benchmarking.m gets my vote, which the macro above
> makes possible, but only for C. Is there something that does the same
> for C# and Java? (browser/listing.m of course does not use either.)

For Java, it is unecessary -- passing an io.MR_TextOutputFile argument to
the report statistics methods will suffice.  (Callers may need to insert
a downcast from MR_MercuryFileStruct, but that should be about it.)

The Java implementation does raise one issue, namely we need to handle
the situation where writing the statistics to the stream throws an
IOException.  (We have to handle it because it's a checked exception.)

The C implementation currently assumes that all the write operations
succeed as well.

I haven't looked at what C# is doing yet (beyond noting that its
statistics currently go to *stdout* for some reason).

Julien.