Re: Correct line/column numbers in byte compiler messages

[Alan Mackenzie]

Hello, Andrea.

On Sat, Mar 21, 2020 at 11:22:03 +0000, Andrea Corallo wrote:
> Alan Mackenzie <address@hidden> writes:

> > It's in the branch scratch/accurate-warning-pos.  The commit which
> > converted the unfinished work to a bug fix was:

> >     commit 2e04ddadab266d245a3bd0f6c19223ea515bdb90
> >     Author: Alan Mackenzie <address@hidden>
> >     Date:   Fri Nov 30 14:55:48 2018 +0000

> >         Sundry amendments to branch scratch/accurate-warning-pos.

> > (except, I think it still outputs two positions for each warning
> > message: the traditional one, and the new correct one).


> I all,

> I've took a very quick look to the accurate-warning-pos and did some
> measures.

Thanks, that's appreciated.

> I've measured the bootstrap time and run elisp-benchmarks (dhrystone
> take out cause broken on both branches) comparing accurate-warning-pos
> against the last in-tree commit it's based on.  Here what I see on my
> dev machine:

> * b071398ba3 @ scratch/accurate-warning-pos

> ** bootstrap

>    real 2m31.076s
>    user 15m8.049s
>    sys  0m38.087s

> ** elisp-benckmarks

>    | test           | non-gc avg (s) | gc avg (s) | gcs avg | tot avg (s) | 
> tot avg err (s) |
>    
> |----------------+----------------+------------+---------+-------------+-----------------|
>    | bubble-no-cons |          11.53 |       0.04 |       4 |       11.57 |   
>          0.01 |
>    | bubble         |           4.74 |       3.81 |     484 |        8.55 |   
>          0.00 |
>    | fibn-rec       |           6.35 |       0.00 |       0 |        6.35 |   
>          0.00 |
>    | fibn-tc        |           5.59 |       0.00 |       0 |        5.59 |   
>          0.02 |
>    | fibn           |          11.90 |       0.00 |       0 |       11.90 |   
>          0.01 |
>    | inclist        |          17.86 |       0.01 |       1 |       17.87 |   
>          0.01 |
>    | listlen-tc     |           6.48 |       0.00 |       0 |        6.48 |   
>          0.01 |
>    | nbody          |           3.58 |       6.70 |     839 |       10.28 |   
>          0.01 |
>    | pidigits       |           5.60 |       5.68 |     457 |       11.28 |   
>          0.03 |
>    
> |----------------+----------------+------------+---------+-------------+-----------------|
>    | total          |          73.62 |      16.24 |    1785 |       89.86 |   
>          0.04 |


> * b619777dd6 (baseline)

> ** bootstrap

>    real 2m20.762s
>    user 13m35.418s
>    sys  0m37.349s

> ** elisp-benckmarks

>    | test           | non-gc avg (s) | gc avg (s) | gcs avg | tot avg (s) | 
> tot avg err (s) |
>    
> |----------------+----------------+------------+---------+-------------+-----------------|
>    | bubble-no-cons |          11.43 |       0.04 |       4 |       11.47 |   
>          0.00 |
>    | bubble         |           4.67 |       3.58 |     487 |        8.25 |   
>          0.01 |
>    | fibn-rec       |           6.21 |       0.00 |       0 |        6.21 |   
>          0.00 |
>    | fibn-tc        |           5.68 |       0.00 |       0 |        5.68 |   
>          0.00 |
>    | fibn           |          11.47 |       0.00 |       0 |       11.47 |   
>          0.00 |
>    | inclist        |          17.37 |       0.01 |       1 |       17.38 |   
>          0.00 |
>    | listlen-tc     |           6.46 |       0.00 |       0 |        6.46 |   
>          0.00 |
>    | nbody          |           3.36 |       6.24 |     839 |        9.60 |   
>          0.01 |
>    | pidigits       |           5.66 |       5.53 |     457 |       11.19 |   
>          0.03 |
>    
> |----------------+----------------+------------+---------+-------------+-----------------|
>    | total          |          72.32 |      15.39 |    1788 |       87.71 |   
>          0.03 |

> The outcome as I see it is that total bootstrap time gets bigger 1.1x
> while normal runtime appears not affected.

Well, it looks like the normal runtime is around 2.x% slower for
scratch/accurate-warning-pos.

> For my quick understanding of how it works this is expected.  The
> additional branch and compare against symbols_with_pos_enabled in `eq'
> is a kind of branch that is very easily predictable by any modern CPU,
> therefore when the feature is off (not compiling) it becomes transparent
> (I'd see a compiler branch hit there too).

In other words, the processor will test symbols_with_pos_enabled
simultaneously with starting the continuation for the "not" case.

This extra test in the EQ code was always the main thing in the slowdown
occurring in this git branch.

When I timed things back in 2018, I got a slowdown of somewhat more than
2.x%.  May I ask what sort of processor you're using?  Mine (unchanged
since then) is an AMD Ryzen.

> elisp-benchmarks are not completely rapresentative for now but
> again... better than nothing.

> Am I missing something else here or we are trading out the exact
> solution for like ~15% off the byte compile-time?  I think this feature
> would be a big step forward for our toolchain opening many
> possibilities.  I suspect fat conses will requires more modifications
> across the whole compilation pipeline (including macros?) bringing a
> less accurate result and still they have to prove the smaller overhead.

> At this point I start suspecting I'm missing something very big here, am
> I?

> Anyway thanks Alan for this.

Thanks!

>   Andrea

> --
> address@hidden

-- 
Alan Mackenzie (Nuremberg, Germany).