Re: Correct line/column numbers in byte compiler messages

[Andrea Corallo <akrl@sdf.org>]

[-- Attachment #1: Type: text/plain, Size: 2600 bytes --]

Alan Mackenzie <acm@muc.de> writes:

> On my Ryzen, I'm seeing a 50% penalty.  :-(  (Admittedly that's
> comparing the year old branch to current master.  I suppose I should
> build the correct comparable revision and try again.)  This suggests
> that the branch prediction logic isn't present (or isn't active) on the
> Ryzen.

This is very strange.  You cerntaly have to compare branches from the
same epoch.  I pretty sure in the last year Paul pushed changes to the
inline policy with some measureble effect on performance.

>> Interestingly with the __builtin_expect trick applied exec time gets
>> back to 50.65s.
>
> How do you do this?  I couldn't make much sense of the documentation of
> __builtin_expect.  :-(

I attach the very simple patch I tried.  Basically the compiler has an
euristic branch predictor (in GCC predict.c) that is used to order the
final basic block output.  The wanted outcome is to have the most likely
execution line as sequential, this on modern CPUs to maximize the
front-end bandwidth.  "__builtin_expect" is just a strong hint to this
predictor.

>> We could probably find a benchmark that better highlights the difference
>> (this is potentially dominated by cache misses while pointer chasing the
>> list) but is it worth?
>
> Could I ask you to do the following timing.
>
> Evaluate the following (e.g. in *scratch*):
>
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> (defmacro time-it (&rest forms)
>   "Time the running of a sequence of forms using `float-time'.
> Call like this: \"M-: (time-it (foo ...) (bar ...) ...)\"."
>   `(let ((start (float-time)))
>     ,@forms
>     (- (float-time) start)))
>
> (defun time-scroll (&optional arg)
>   (interactive "P")
>   (message "%s"
>            (time-it
>             (condition-case nil
>                 (while t
>                   (if arg (scroll-down) (scroll-up))
>                   (sit-for 0))
>               (error nil)))))
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>
> , visit .../emacs/src/xdisp.c, and do M-: (time-scroll).  This scrolls
> through the buffer and prints a timing in the minibuffer.  (N.B. to run
> this again, type something at BOB and undo it, thus marking the
> fontification as stale.)
>
> I'm seeing 19.4s vs. 22.2s, which is around 15% difference.  :-(

I get 19.30 sec against 16.65 that is 15% difference here too.  This is
extremely interesting and would be worth profiling.

I bet on the GC for this! (Note I'm notoriously wrong when speculating
on benchmarks :)

Regards

  Andrea
  
-- 
akrl@sdf.org


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: comp-hint.patch --]
[-- Type: text/x-diff, Size: 1572 bytes --]

diff --git a/src/lisp.h b/src/lisp.h
index a22043026a..6e3cca1bbc 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -394,8 +394,12 @@ typedef EMACS_INT Lisp_Word;
 /* #define lisp_h_EQ(x, y) (XLI (x) == XLI (y)) */
 
 /* verify (NIL_IS_ZERO) */
+
+#define SYMBOLS_WITH_POS_ENABLED			\
+  __builtin_expect(symbols_with_pos_enabled, 0)
+
 #define lisp_h_EQ(x, y) ((XLI ((x)) == XLI ((y)))       \
-  || (symbols_with_pos_enabled    \
+  || (SYMBOLS_WITH_POS_ENABLED    \
   && (SYMBOL_WITH_POS_P ((x))                        \
       ? BARE_SYMBOL_P ((y))                               \
         ? (XSYMBOL_WITH_POS((x)))->sym == (y)          \
@@ -424,7 +428,7 @@ typedef EMACS_INT Lisp_Word;
 #define lisp_h_BARE_SYMBOL_P(x) TAGGEDP ((x), Lisp_Symbol)
 /* verify (NIL_IS_ZERO) */
 #define lisp_h_SYMBOLP(x) ((BARE_SYMBOL_P ((x)) ||               \
-                            (symbols_with_pos_enabled && (SYMBOL_WITH_POS_P ((x))))))
+                            (SYMBOLS_WITH_POS_ENABLED && (SYMBOL_WITH_POS_P ((x))))))
 #define lisp_h_TAGGEDP(a, tag) \
    (! (((unsigned) (XLI (a) >> (USE_LSB_TAG ? 0 : VALBITS)) \
 	- (unsigned) (tag)) \
@@ -463,7 +467,7 @@ typedef EMACS_INT Lisp_Word;
 /* verify (NIL_IS_ZERO) */
 # define lisp_h_XSYMBOL(a)                      \
      (eassert (SYMBOLP ((a))),                      \
-      (!symbols_with_pos_enabled             \
+      (!SYMBOLS_WITH_POS_ENABLED             \
       ? (XBARE_SYMBOL ((a)))             \
        : (BARE_SYMBOL_P ((a)))           \
       ? (XBARE_SYMBOL ((a)))                                    \