Discussion:
[m-dev.] --warn-non-tail-recursion and deep profiling
Julien Fischer
2016-03-01 23:13:24 UTC
Permalink
Hi,

Was it intended that --warn-non-tail-recursion should emit warnings
in deep profiling grades? Since those grades disable tail recursion
you obviously get a warning about every recursive predicate.

Julien.
Paul Bone
2016-03-02 00:52:34 UTC
Permalink
Post by Julien Fischer
Hi,
Was it intended that --warn-non-tail-recursion should emit warnings
in deep profiling grades? Since those grades disable tail recursion
you obviously get a warning about every recursive predicate.
Not specifically.

Because this is disabled by default simply enabling deep profiling won't
cause these warnings. A user giving both options on the command line will
very quickly figure out what is happening, but if --warn-non-tail-recursion
is specified in a Mercury.options file, especially if it's setup by another
developer on a team, then enabling --deep-profiling might cause some
surprise. I think that the question is: which surprise is better? Lots of
warnings or running out of stack space. Does the answer to this change if
we take away the historical behaviour of breaking tail recursion in deep
profiling grades?
--
Paul Bone
Julien Fischer
2016-03-02 01:33:22 UTC
Permalink
Hi Paul,
Post by Paul Bone
Post by Julien Fischer
Hi,
Was it intended that --warn-non-tail-recursion should emit warnings
in deep profiling grades? Since those grades disable tail recursion
you obviously get a warning about every recursive predicate.
Not specifically.
Because this is disabled by default simply enabling deep profiling won't
cause these warnings. A user giving both options on the command line will
very quickly figure out what is happening, but if --warn-non-tail-recursion
is specified in a Mercury.options file, especially if it's setup by another
developer on a team, then enabling --deep-profiling might cause some
surprise.
That is the case I'm looking at. Specifically, I'm looking at the
"failure" of the test case invalid/require_tailrec_2 in the grade
none.gc.profdeep.trseg.stseg. We get additional warnings for this test
case because tail recursion is disabled. There are various ways to
avoid this failure (e.g. extra expected output, don't run the test in
profdeep grades); I'm trying to determine which one to use here.
Post by Paul Bone
I think that the question is: which surprise is better? Lots of
warnings
It's lots of warnings that are *not* present in non profdeep grades.
Also, in the profdeep grades, they are *useless* warnings since there's
nothing you can do about them. (Presuambly, --warn-non-tail-recursion
respects --halt-at-warn, which is going to be a problem in profdeep
grades if the --halt-at-warn is also present in your Mercury.options
file.)
Post by Paul Bone
or running out of stack space.
That won't come as a surprise since I've read the user's guide
and that's what it says will happen!
Post by Paul Bone
Does the answer to this change if we take away the historical
behaviour of breaking tail recursion in deep profiling grades?
How do you propose to do that? We do have an undocumented option
for allowing (some) tail recursion in deep profiling grades, however
we currently say the following about it:

% We do not currently enable (or publicly document) this option
% because its use results in significant overheads. Also, it is not
% compatible with coverage profiling, which is enabled by default. By
% default, all deep profiling grades are also built with
% --stack-segments in order to avoid problems caused by the lack of
% tail recursion.

(I suspect it's actually broken due to bitrot in any case.)

Julien.
Paul Bone
2016-03-02 02:00:59 UTC
Permalink
Post by Julien Fischer
Hi Paul,
Post by Paul Bone
Post by Julien Fischer
Hi,
Was it intended that --warn-non-tail-recursion should emit warnings
in deep profiling grades? Since those grades disable tail recursion
you obviously get a warning about every recursive predicate.
Not specifically.
Because this is disabled by default simply enabling deep profiling won't
cause these warnings. A user giving both options on the command line will
very quickly figure out what is happening, but if --warn-non-tail-recursion
is specified in a Mercury.options file, especially if it's setup by another
developer on a team, then enabling --deep-profiling might cause some
surprise.
That is the case I'm looking at. Specifically, I'm looking at the
"failure" of the test case invalid/require_tailrec_2 in the grade
none.gc.profdeep.trseg.stseg. We get additional warnings for this test
case because tail recursion is disabled. There are various ways to
avoid this failure (e.g. extra expected output, don't run the test in
profdeep grades); I'm trying to determine which one to use here.
I suggest not running these tests with the profdeep grade component.
Post by Julien Fischer
Post by Paul Bone
I think that the question is: which surprise is better? Lots of
warnings
It's lots of warnings that are *not* present in non profdeep grades.
Also, in the profdeep grades, they are *useless* warnings since there's
nothing you can do about them. (Presuambly, --warn-non-tail-recursion
respects --halt-at-warn, which is going to be a problem in profdeep
grades if the --halt-at-warn is also present in your Mercury.options
file.)
Uses can:
* Be aware that their programs may crash.
* When they crash, more quickly decide to increase heap space or use
stack segments.
* Turn off deep profiling.
Post by Julien Fischer
Post by Paul Bone
or running out of stack space.
That won't come as a surprise since I've read the user's guide
and that's what it says will happen!
You and I are not a fair test of user-friendlyness. We're too aware of how
Mercury and deep profiling works.

Another option is if deep profiling and --warn-non-tail-recursive are both
enabled, emit one general warning only. The text of the warning message
could even change depending on the stseg grade component.
Post by Julien Fischer
Post by Paul Bone
Does the answer to this change if we take away the historical
behaviour of breaking tail recursion in deep profiling grades?
How do you propose to do that? We do have an undocumented option
for allowing (some) tail recursion in deep profiling grades, however
% We do not currently enable (or publicly document) this option
% because its use results in significant overheads. Also, it is not
% compatible with coverage profiling, which is enabled by default. By
% default, all deep profiling grades are also built with
% --stack-segments in order to avoid problems caused by the lack of
% tail recursion.
(I suspect it's actually broken due to bitrot in any case.)
Yes, I think this is broken.

That's not what I meant anyway (sorry I didn't explain it clearly). I'm not
proposing changing deep profiling. I'm asking if the answer to this
question changes if we consider new users who aren't aware that deep
profiling breaks tail recursion.
--
Paul Bone
Julien Fischer
2016-03-02 02:22:27 UTC
Permalink
Post by Paul Bone
Post by Julien Fischer
Hi Paul,
Post by Paul Bone
Post by Julien Fischer
Hi,
Was it intended that --warn-non-tail-recursion should emit warnings
in deep profiling grades? Since those grades disable tail recursion
you obviously get a warning about every recursive predicate.
Not specifically.
Because this is disabled by default simply enabling deep profiling won't
cause these warnings. A user giving both options on the command line will
very quickly figure out what is happening, but if --warn-non-tail-recursion
is specified in a Mercury.options file, especially if it's setup by another
developer on a team, then enabling --deep-profiling might cause some
surprise.
That is the case I'm looking at. Specifically, I'm looking at the
"failure" of the test case invalid/require_tailrec_2 in the grade
none.gc.profdeep.trseg.stseg. We get additional warnings for this test
case because tail recursion is disabled. There are various ways to
avoid this failure (e.g. extra expected output, don't run the test in
profdeep grades); I'm trying to determine which one to use here.
I suggest not running these tests with the profdeep grade component.
At least part of that test involves the warning about a
require_tail_recursion pragma on a non-recursive predicate. That at
least, ought to work in profdeep grades. I'll add an alternative
expected output for this one.

...
Post by Paul Bone
Another option is if deep profiling and --warn-non-tail-recursive are both
enabled, emit one general warning only. The text of the warning message
could even change depending on the stseg grade component.
Rather than a warning, I think a single informational message (per
affected module) along those lines would be better.

Julien.
Zoltan Somogyi
2016-03-02 05:34:09 UTC
Permalink
Post by Paul Bone
Post by Julien Fischer
That is the case I'm looking at. Specifically, I'm looking at the
"failure" of the test case invalid/require_tailrec_2 in the grade
none.gc.profdeep.trseg.stseg. We get additional warnings for this test
case because tail recursion is disabled. There are various ways to
avoid this failure (e.g. extra expected output, don't run the test in
profdeep grades); I'm trying to determine which one to use here.
I suggest not running these tests with the profdeep grade component.
I am pretty sure that a proper solution of this problem has two parts.
The above (not running these tests in grades that intrinsically destroy
tail recursion) is one.
Post by Paul Bone
Another option is if deep profiling and --warn-non-tail-recursive are both
enabled, emit one general warning only. The text of the warning message
could even change depending on the stseg grade component.
And this is the other.

Zoltan.

Loading...