Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Stefan Monnier]

> branch: emacs-26
[...]
>     Speed up (format "%s" STRING) and the like

Shouldn't this have been committed to `master` instead?


        Stefan

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Paul Eggert]

Stefan Monnier wrote:
> Shouldn't this have been committed to `master` instead?

Either branch is fine with me. I committed it to emacs-26 because that's what 
Eli suggested.

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Wed, 4 Oct 2017 23:07:35 -0700
> Cc: emacs-devel@gnu.org
> 
> Stefan Monnier wrote:
> > Shouldn't this have been committed to `master` instead?
> 
> Either branch is fine with me. I committed it to emacs-26 because that's what 
> Eli suggested.

My reasoning was that the change can be considered a bug fix, as it
fixes a discrepancy between the docs and the actual implementation;
and we didn't yet have our first pretest from the release branch, so
minor improvements are still okay.

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Stefan Monnier]

> My reasoning was that the change can be considered a bug fix, as it
> fixes a discrepancy between the docs and the actual implementation;

Ah, OK.  The commit message suggested it was only a performance
improvement, which is why I thought it odd for it to go to emacs-26.
Sorry 'bout the noise,


        Stefan

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Kaushal Modi]

[-- Attachment #1.1: Type: text/plain, Size: 931 bytes --]

Is it possible this would cause the echo area to be 3-lines high? Also
noticed that the hydra.el[1] package is unusable after the latest build
from emacs-26. My last good build was just before the commit related to
format change.

hydra.el gives this error:

[image: image.png]
Even deleting and reinstalling that package (thus recompiling) does not fix
that. Note that the hydra package hasn't changed in this duration.. I just
rebuilt from emacs-26.

[1]: https://elpa.gnu.org/packages/hydra.html

On Thu, Oct 5, 2017 at 9:20 AM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > My reasoning was that the change can be considered a bug fix, as it
> > fixes a discrepancy between the docs and the actual implementation;
>
> Ah, OK.  The commit message suggested it was only a performance
> improvement, which is why I thought it odd for it to go to emacs-26.
> Sorry 'bout the noise,
>
>
>         Stefan
>
> --

Kaushal Modi

[-- Attachment #1.2: Type: text/html, Size: 1545 bytes --]

[-- Attachment #2: image.png --]
[-- Type: image/png, Size: 16870 bytes --]

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Kaushal Modi]

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

On Thu, Oct 5, 2017 at 12:57 PM Kaushal Modi <kaushal.modi@gmail.com> wrote:

> Is it possible this would cause the echo area to be 3-lines high? Also
> noticed that the hydra.el package is unusable after the latest build from
> emacs-26. My last good build was just before the commit related to format
> change.
>

OK, I can now confirm that. Reverting just that commit[1] brings things
back to normal.

So it should be reverted on emacs-26 for now?

[1]:
http://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-26&id=b5c965dbd8eef12e8e7c79e59d2fa58c2b8d60d7
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1143 bytes --]

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Kaushal Modi]

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

On Thu, Oct 5, 2017 at 1:08 PM Kaushal Modi <kaushal.modi@gmail.com> wrote:

> On Thu, Oct 5, 2017 at 12:57 PM Kaushal Modi <kaushal.modi@gmail.com>
> wrote:
>
>> Is it possible this would cause the echo area to be 3-lines high? Also
>> noticed that the hydra.el package is unusable after the latest build from
>> emacs-26. My last good build was just before the commit related to format
>> change.
>>
>
> OK, I can now confirm that. Reverting just that commit[1] brings things
> back to normal.
>
> So it should be reverted on emacs-26 for now?
>

Argh! Referenced the wrong commit.. I meant this format-related commit:
http://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-26&id=3db388b0bf83d3138562f09ce25fab8ba89bcc81

-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1545 bytes --]

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Eli Zaretskii]

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Thu, 05 Oct 2017 17:08:06 +0000
> Cc: Paul Eggert <eggert@cs.ucla.edu>, emacs-devel@gnu.org, oleh@oremacs.com
> 
>  Is it possible this would cause the echo area to be 3-lines high? Also noticed that the hydra.el package is
>  unusable after the latest build from emacs-26. My last good build was just before the commit related to
>  format change.
> 
> OK, I can now confirm that. Reverting just that commit[1] brings things back to normal.

Please always provide a minimal recipe to reproduce whatever problems
you see.  For add-on packages, please also provide an explanation why
what you see is problematic.  E.g., in this case, by just seeing the
echo-area messages, I don't quite understand the problem, as I don't
use hydra.

Some minimal details like that are always necessary to understand what
is the problem and how to fix it.

> So it should be reverted on emacs-26 for now?

We don't usually revert commits when they cause problems, we rather
prefer fixing the problems.  I'm quite sure this one will be fixed
soon enough.

Thanks.

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Kaushal Modi]

[-- Attachment #1.1: Type: text/plain, Size: 3365 bytes --]

On Thu, Oct 5, 2017 at 2:48 PM Eli Zaretskii <eliz@gnu.org> wrote:

>
> Please always provide a minimal recipe to reproduce whatever problems
> you see.  For add-on packages, please also provide an explanation why
> what you see is problematic.  E.g., in this case, by just seeing the
> echo-area messages, I don't quite understand the problem, as I don't
> use hydra.
>

Understood, here is the recipe.

1. Install hydra from GNU Elpa in emacs -Q.
2. Evaluate the below

  (defhydra hydra-test (:color teal
                        :columns 7)
    "Test Hydra"
    ("<SPC>"   (message "Pressed space") "Space")
    ("<s-SPC>" (message "Pressed super space") "Super Space")
    ("q" (message "Pressed q") "q")
    ("r" (message "Pressed r") "r")
    ("s" (message "Pressed s") "s")
    ("t" (message "Pressed t") "t")
    ("u" (message "Pressed u") "u")
    ("v" (message "Pressed v") "v")
    ("w" (message "Pressed w") "w")
    ("x" (message "Pressed x") "x")
    ("y" (message "Pressed y") "y")
    ("z" (message "Pressed z") "z")
    ("A" (message "Pressed A") "A")
    ("B" (message "Pressed B") "B")
    ("C" (message "Pressed C") "C")
    ("D" (message "Pressed D") "D")
    ("E" (message "Pressed E") "E")
    ("F" (message "Pressed F") "F")
    ("G" (message "Pressed G") "G")
    ("H" (message "Pressed H") "H")
    ("I" (message "Pressed I") "I")
    ("J" (message "Pressed J") "J")
    ("K" (message "Pressed K") "K")
    ("L" (message "Pressed L") "L")
    ("M" (message "Pressed M") "M")
    ("N" (message "Pressed N") "N")
    ("O" (message "Pressed O") "O")
    ("P" (message "Pressed P") "P")
    ("Q" (message "Pressed Q") "Q")
    ("R" (message "Pressed R") "R")
    ("S" (message "Pressed S") "S")
    ("T" (message "Pressed T") "T")
    ("U" (message "Pressed U") "U")
    ("V" (message "Pressed V") "V")
    ("W" (message "Pressed W") "W")
    ("X" (message "Pressed X") "X")
    ("Y" (message "Pressed Y") "Y")
    ("Z" (message "Pressed Z") "Z")
    ("C-g"     nil "cancel" :color blue))
  (global-set-key (kbd "C-c l") #'hydra-test/body)
3. Press "C-c l"

I cannot quite place what's special about the above test hydra, but once
you press C-c l, you will see:

[image: image.png]

and the hydra will be unresponsive.. you won't be able to exist that Hydra
window at the bottom (of course, you can kill it.. but the hydra won't be
possible to quit any more).

I have copied Oleh so that he can provide more insight into how this format
commit affects hydra, and what that "binding stack not balanced" error
means.

About "something special about the test hydra":
- I do not get this error if I reduce the number of elements/heads of that
test hydra.
- I do not get this error if I remove the head with "<s-SPC>" binding:
("<s-SPC>" (message "Pressed super space") "Super Space")

I arrive to above test hydra only by trial.

Another issue caused by the format speed-up commit, that I cannot recreate
in a recipe is that once emacs started up with my config, I would see the
echo area 3-line tall, which is odd, and I've never seen.


> We don't usually revert commits when they cause problems, we rather
> prefer fixing the problems.  I'm quite sure this one will be fixed
> soon enough.
>

Sure. I just thought this commit was too experimental, as it made my emacs
session 100% unusable, and rebuilding was my only option.
-- 

Kaushal Modi

[-- Attachment #1.2: Type: text/html, Size: 6097 bytes --]

[-- Attachment #2: image.png --]
[-- Type: image/png, Size: 46038 bytes --]

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Paul Eggert]

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

I could not reproduce the problem, via this recipe on Fedora 26 x86-64:

   wget https://elpa.gnu.org/packages/hydra-0.14.0.tar
   tar xf hydra-0.14.0.tar
   cd hydra-0.14.0
   make clean
   make emacs="$HOME/src/gnu/emacs/emacs-tmp1/src/emacs -Q" run
   [cut and paste the code in:
http://lists.gnu.org/archive/html/emacs-devel/2017-10/msg00160.html
   ]
   [C-x C-e after both expressions in that code]
   C-c l

where emacs-tmp1 is just after the commit in question 
(3db388b0bf83d3138562f09ce25fab8ba89bcc81), built from a fresh git 
checkout. I do not see the "binding stack not balanced" message.

Can you try the same thing as the above, and if you see the problem then 
let us know what .elc files you get? The .elc files I got are in the 
attached tarball.

What platform are you using?

[-- Attachment #2: hydra-elc.tar.gz --]
[-- Type: application/gzip, Size: 32534 bytes --]

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Kaushal Modi]

[-- Attachment #1.1: Type: text/plain, Size: 10237 bytes --]

On Thu, Oct 5, 2017 at 5:44 PM Paul Eggert <eggert@cs.ucla.edu> wrote:

> I could not reproduce the problem, via this recipe on Fedora 26 x86-64:
>
>    wget https://elpa.gnu.org/packages/hydra-0.14.0.tar
>    tar xf hydra-0.14.0.tar
>    cd hydra-0.14.0
>    make clean
>    make emacs="$HOME/src/gnu/emacs/emacs-tmp1/src/emacs -Q" run
>    [cut and paste the code in:
> http://lists.gnu.org/archive/html/emacs-devel/2017-10/msg00160.html
>    ]
>    [C-x C-e after both expressions in that code]
>    C-c l
>
> where emacs-tmp1 is just after the commit in question
> (3db388b0bf83d3138562f09ce25fab8ba89bcc81), built from a fresh git
> checkout. I do not see the "binding stack not balanced" message.
>

That's indeed strange.. I cannot recreate the error if I do it *that* way.
But I can recreate the error when building hydra through package.el.

Please try the below method now and see if that helps you recreate the
issue:

- You just need to launch emacs -Q on the failing build, eval the whole
thing below and C-c l
- Of course the code says it all, but for clarity, this will install the
hydra package from GNU Elpa in your /tmp dir. So your default emacs
config/elpa won't be touched.

=====
(defmacro emacs-pkg-debug-setup (pkg-alist &rest body)
  "Install packages in PKG-ALIST and evaluate BODY.
Each element of PKG-ALIST has the form (((ID . LOCATION) . (PKG1 PKG2 ..))
..).
The ID and LOCATION are the same as the ones in `package-archives'.
PKG1, PKG2, .. are package names from the ID archive.

Example usage:

1. Launch 'emacs -Q'.
2. Copy this macro definition to its scratch buffer and evaluate it.
3. Evaluate a minimum working example using this macro as below:
     (emacs-pkg-debug-setup '(;; Install hydra from GNU Elpa
                              (nil . (hydra))
                              ;; Install org from Org Elpa
                              ((\"org\" . \"http://orgmode.org/elpa/\") .
(org)))
       ;; Then evaluate the below forms
       (org-mode)) "
  (declare (indent 1) (debug t))
  `(progn
     (require 'package)
     (setq user-emacs-directory (concat temporary-file-directory
                                        (getenv "USER") "/"
                                        ".emacs.d-debug/"))
     (setq package-user-dir (format "%selpa_%s/"
                                    user-emacs-directory
                                    emacs-major-version))
     (let (archive pkgs)
       (dolist (archive-alist ,pkg-alist)
         (setq archive (car archive-alist))
         (when archive
           (add-to-list 'package-archives archive :append))
         (setq pkgs (append pkgs (cdr archive-alist))))
       (package-initialize)
       (package-refresh-contents)

       (dolist (pkg pkgs)
         (when (and pkg (not (package-installed-p pkg)))
           (package-install pkg))
         (require pkg))

       ,@body)))

(emacs-pkg-debug-setup '((nil . (hydra)))
  (defhydra hydra-test (:color teal
                        :columns 7)
    "Test Hydra"
    ("<SPC>"   (message "Pressed space") "Space")
    ("<s-SPC>" (message "Pressed super space") "Super Space")
    ("q" (message "Pressed q") "q")
    ("r" (message "Pressed r") "r")
    ("s" (message "Pressed s") "s")
    ("t" (message "Pressed t") "t")
    ("u" (message "Pressed u") "u")
    ("v" (message "Pressed v") "v")
    ("w" (message "Pressed w") "w")
    ("x" (message "Pressed x") "x")
    ("y" (message "Pressed y") "y")
    ("z" (message "Pressed z") "z")
    ("A" (message "Pressed A") "A")
    ("B" (message "Pressed B") "B")
    ("C" (message "Pressed C") "C")
    ("D" (message "Pressed D") "D")
    ("E" (message "Pressed E") "E")
    ("F" (message "Pressed F") "F")
    ("G" (message "Pressed G") "G")
    ("H" (message "Pressed H") "H")
    ("I" (message "Pressed I") "I")
    ("J" (message "Pressed J") "J")
    ("K" (message "Pressed K") "K")
    ("L" (message "Pressed L") "L")
    ("M" (message "Pressed M") "M")
    ("N" (message "Pressed N") "N")
    ("O" (message "Pressed O") "O")
    ("P" (message "Pressed P") "P")
    ("Q" (message "Pressed Q") "Q")
    ("R" (message "Pressed R") "R")
    ("S" (message "Pressed S") "S")
    ("T" (message "Pressed T") "T")
    ("U" (message "Pressed U") "U")
    ("V" (message "Pressed V") "V")
    ("W" (message "Pressed W") "W")
    ("X" (message "Pressed X") "X")
    ("Y" (message "Pressed Y") "Y")
    ("Z" (message "Pressed Z") "Z")
    ("C-g"     nil "cancel" :color blue))
  (global-set-key (kbd "C-c l") #'hydra-test/body)
  (message "Done!"))
=====


> Can you try the same thing as the above, and if you see the problem then
> let us know what .elc files you get? The .elc files I got are in the
> attached tarball.
>

Thanks. I did ediff of my elc files vs yours and there were no differences!
Probably ediff ignores byte-codes? I did not compare the two sets of files
closely.

I have attached tar.xz of my .elc files. Hope that holds the key to this
problem, especially the lv.elc.

I should have also sent the full error backtrace earlier.. here it is now:

Debugger entered--Lisp error: (error "binding stack not balanced (serious
byte compiler bug)")
  lv-message(#("Test Hydra:\n  <SPC>: Space       <s-SPC>: Super Space
 q: q                 r: r                 s: s                 t: t
         u: u\n      v: v                 w: w                 x: x
         y: y                 z: z                 A: A                 B:
B\n      C: C                 D: D                 E: E                 F:
F                 G: G                 H: H                 I: I\n      J:
J                 K: K                 L: L                 M: M
     N: N                 O: O                 P: P\n      Q: Q
     R: R                 S: S                 T: T                 U: U
             V: V                 W: W\n      X: X                 Y: Y
             Z: Z               C-g: cancel" 14 19 (face hydra-face-teal)
33 40 (face hydra-face-teal) 60 61 (face hydra-face-teal) 81 82 (face
hydra-face-teal) 102 103 (face hydra-face-teal) 123 124 (face
hydra-face-teal) 144 145 (face hydra-face-teal) 155 156 (face
hydra-face-teal) 176 177 (face hydra-face-teal) 197 198 (face
hydra-face-teal) 218 219 (face hydra-face-teal) 239 240 (face
hydra-face-teal) 260 261 (face hydra-face-teal) 281 282 (face
hydra-face-teal) 292 293 (face hydra-face-teal) 313 314 (face
hydra-face-teal) 334 335 (face hydra-face-teal) 355 356 (face
hydra-face-teal) 376 377 (face hydra-face-teal) 397 398 (face
hydra-face-teal) 418 419 (face hydra-face-teal) 429 430 (face
hydra-face-teal) 450 451 (face hydra-face-teal) 471 472 (face
hydra-face-teal) 492 493 (face hydra-face-teal) 513 514 (face
hydra-face-teal) 534 535 (face hydra-face-teal) 555 556 (face
hydra-face-teal) 566 567 (face hydra-face-teal) 587 588 (face
hydra-face-teal) 608 609 (face hydra-face-teal) 629 630 (face
hydra-face-teal) 650 651 (face hydra-face-teal) 671 672 (face
hydra-face-teal) 692 693 (face hydra-face-teal) 703 704 (face
hydra-face-teal) 724 725 (face hydra-face-teal) 745 746 (face
hydra-face-teal) 764 767 (face hydra-face-teal)))
  hydra-show-hint((format #("Test Hydra:\n  <SPC>: Space       <s-SPC>:
Super Space       q: q                 r: r                 s: s
     t: t                 u: u\n      v: v                 w: w
     x: x                 y: y                 z: z                 A: A
             B: B\n      C: C                 D: D                 E: E
             F: F                 G: G                 H: H
 I: I\n      J: J                 K: K                 L: L
 M: M                 N: N                 O: O                 P: P\n
Q: Q                 R: R                 S: S                 T: T
         U: U                 V: V                 W: W\n      X: X
         Y: Y                 Z: Z               C-g: cancel" 14 19 (face
hydra-face-teal) 33 40 (face hydra-face-teal) 60 61 (face hydra-face-teal)
81 82 (face hydra-face-teal) 102 103 (face hydra-face-teal) 123 124 (face
hydra-face-teal) 144 145 (face hydra-face-teal) 155 156 (face
hydra-face-teal) 176 177 (face hydra-face-teal) 197 198 (face
hydra-face-teal) 218 219 (face hydra-face-teal) 239 240 (face
hydra-face-teal) 260 261 (face hydra-face-teal) 281 282 (face
hydra-face-teal) 292 293 (face hydra-face-teal) 313 314 (face
hydra-face-teal) 334 335 (face hydra-face-teal) 355 356 (face
hydra-face-teal) 376 377 (face hydra-face-teal) 397 398 (face
hydra-face-teal) 418 419 (face hydra-face-teal) 429 430 (face
hydra-face-teal) 450 451 (face hydra-face-teal) 471 472 (face
hydra-face-teal) 492 493 (face hydra-face-teal) 513 514 (face
hydra-face-teal) 534 535 (face hydra-face-teal) 555 556 (face
hydra-face-teal) 566 567 (face hydra-face-teal) 587 588 (face
hydra-face-teal) 608 609 (face hydra-face-teal) 629 630 (face
hydra-face-teal) 650 651 (face hydra-face-teal) 671 672 (face
hydra-face-teal) 692 693 (face hydra-face-teal) 703 704 (face
hydra-face-teal) 724 725 (face hydra-face-teal) 745 746 (face
hydra-face-teal) 764 767 (face hydra-face-teal))) hydra-test)
  hydra-test/body()
  funcall-interactively(hydra-test/body)
  call-interactively(hydra-test/body nil nil)
  command-execute(hydra-test/body)



> What platform are you using?
>

OS: RHEL 6.6

Here is my detailed emacs version info (I just rebuilt using the failing
commit 3db388b0 directly):

Emacs version: GNU Emacs 26.0.60 (build 19, x86_64-pc-linux-gnu, GTK+
Version 2.24.23)
 of 2017-10-05, built using commit 3db388b0bf83d3138562f09ce25fab8ba89bcc81.

./configure options:
  --with-modules
--prefix=/home/kmodi/usr_local/apps/6/emacs/3db388_format_speedup
'--program-transform-name=s/^ctags$/ctags_emacs/'
'CPPFLAGS=-I/home/kmodi/usr_local/6/include -I/usr/include/freetype2
-I/usr/include' 'CFLAGS=-ggdb3 -O0' 'CXXFLAGS=-ggdb3 -O0'
'LDFLAGS=-L/home/kmodi/usr_local/6/lib -L/home/kmodi/usr_local/6/lib64
-ggdb3'

Features:
  XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY
ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS
GTK2 X11 MODULES

Thanks.

PS: If the above emacs-pkg-debug-setup helps out, it will be great if it
can be included in emacs (so that I don't need to paste the whole macro
each time :P)
-- 

Kaushal Modi

[-- Attachment #1.2: Type: text/html, Size: 15036 bytes --]

[-- Attachment #2: hydra-built-using-package-emacs-26.tar.xz --]
[-- Type: application/octet-stream, Size: 27004 bytes --]

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Paul Eggert]

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

On 10/05/2017 05:47 PM, Kaushal Modi wrote:
> Please try the below method now and see if that helps you recreate the 
> issue:

Thanks, that did the trick. I reproduced the problem on an RHEL 6.9 
platform. The problem was due to a significant typo in one of my earlier 
commits, a typo that was exposed by the commit that you mentioned. To 
fix it, I installed the attached patch into emacs-26 and merged emacs-26 
to master.


[-- Attachment #2: 0001-Fix-bug-in-recent-styled_format-change.patch --]
[-- Type: text/x-patch, Size: 2213 bytes --]

From 9226cf325421a168b42bd27abf5e171e877b48b9 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 6 Oct 2017 10:32:46 -0700
Subject: [PATCH] Fix bug in recent styled_format change

Problem reported by Kaushal Modi in:
http://lists.gnu.org/archive/html/emacs-devel/2017-10/msg00141.html
* src/editfns.c (styled_format): Fix bug where USE_SAFE_ALLOCA was
not always followed by SAFE_FREE.  This bug was introduced in my
patch 2017-09-26T23:31:57Z!eggert@cs.ucla.edu entitled "Avoid some
unnecessary copying in Fformat etc."
---
 src/editfns.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/editfns.c b/src/editfns.c
index d88a913c66..e65bd34da8 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -4179,6 +4179,7 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
      multibyte character of the previous string.  This flag tells if we
      must consider such a situation or not.  */
   bool maybe_combine_byte;
+  Lisp_Object val;
   bool arg_intervals = false;
   USE_SAFE_ALLOCA;
   sa_avail -= sizeof initial_buffer;
@@ -4417,7 +4418,10 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 	    {
 	      if (format == end && format - format_start == 2
 		  && ! string_intervals (args[0]))
-		return arg;
+		{
+		  val = arg;
+		  goto return_val;
+		}
 
 	      /* handle case (precision[n] >= 0) */
 
@@ -4862,11 +4866,14 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
     emacs_abort ();
 
   if (! new_result)
-    return args[0];
+    {
+      val = args[0];
+      goto return_val;
+    }
 
   if (maybe_combine_byte)
     nchars = multibyte_chars_in_text ((unsigned char *) buf, p - buf);
-  Lisp_Object val = make_specified_string (buf, nchars, p - buf, multibyte);
+  val = make_specified_string (buf, nchars, p - buf, multibyte);
 
   /* If the format string has text properties, or any of the string
      arguments has text properties, set up text properties of the
@@ -4964,6 +4971,7 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 	    }
     }
 
+ return_val:
   /* If we allocated BUF or INFO with malloc, free it too.  */
   SAFE_FREE ();
 
-- 
2.13.6

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Kaushal Modi]

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

On Fri, Oct 6, 2017 at 1:45 PM Paul Eggert <eggert@cs.ucla.edu> wrote:

>
> Thanks, that did the trick. I reproduced the problem on an RHEL 6.9
> platform. The problem was due to a significant typo in one of my earlier
> commits, a typo that was exposed by the commit that you mentioned. To
> fix it, I installed the attached patch into emacs-26 and merged emacs-26
> to master.
>

Awesome! I'll try the latest commit soon.

BTW what would be the simplest way to explain why it worked using the
Makefile in hydra pkg, but not when installed via pacakge.el? Inspite of
same (right?) .elc files?

I looked at the commit, but being C-illiterate, made no sense to me.

PS:
Also shouldn't bug reports be easier to write (and recipes easy to
reproduce) if that macro be made part of emacs core? So that people can
assimilate info about packages needing to be installed, followed by elisp
forms in a concise fashion? Eli?
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1415 bytes --]

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Paul Eggert]

On 10/06/2017 10:49 AM, Kaushal Modi wrote:
> what would be the simplest way to explain why it worked using the 
> Makefile in hydra pkg, but not when installed via pacakge.el? Inspite 
> of same (right?) .elc files?

The .elc files were not the same. However, I think that was a blind 
alley, as .elc differences can be benign. The diagnostic was output only 
when (format FOO) was called with a long FOO (greater than 235 chars, 
roughly) without any % directives in it, and where the (format FOO) call 
was either directly from bytecode or indirectly via channels that never 
used unbind_to. The Makefile approach didn't byte-compile one of the 
files that the package.el approach compiled, and I suspect that the 
different path to (lv-message "long format with no % directives") 
explained why I didn't reproduce the bug at first.

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Kaushal Modi]

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

On Fri, Oct 6, 2017 at 2:06 PM Paul Eggert <eggert@cs.ucla.edu> wrote:

> The .elc files were not the same. However, I think that was a blind
> alley, as .elc differences can be benign. The diagnostic was output only
> when (format FOO) was called with a long FOO (greater than 235 chars,
> roughly) without any % directives in it, and where the (format FOO) call
> was either directly from bytecode or indirectly via channels that never
> used unbind_to. The Makefile approach didn't byte-compile one of the
> files that the package.el approach compiled, and I suspect that the
> different path to (lv-message "long format with no % directives")
> explained why I didn't reproduce the bug at first.
>

Thanks. That explains why the hydra needed to have that many elements.. to
create the long lv-message. But does it also explain this observation from
my earlier email:

> I do not get this error if I remove the head with "<s-SPC>" binding:
("<s-SPC>" (message "Pressed super space") "Super Space")

BTW I confirm the fix. Thanks!
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1634 bytes --]

Re: [Emacs-diffs] emacs-26 3db388b: Speed up (format "%s" STRING) and the like

[Kaushal Modi]

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

On Fri, Oct 6, 2017, 2:15 PM Kaushal Modi <kaushal.modi@gmail.com> wrote:

> But does it also explain this observation from my earlier email:
>
> > I do not get this error if I remove the head with "<s-SPC>" binding:
> ("<s-SPC>" (message "Pressed super space") "Super Space")
>

Actually, I just realized. That has nothing to do with the < or > chars,
just that it added a lot more chars to the lv-message compared to the other
hydra elements.

> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1357 bytes --]