pcase-memoize: equal first branch, yet different

pcase-memoize: equal first branch, yet different

[Michael Heerdegen]

Hi,

I get a not so useful message using `pcase' in the following scenario
using trunk:

I want to control "dired-sort-menu.el" from the mode-line in dired
buffers.  I want the mode-line to display diverse "ls" flags, the
enabled flags are highlighted, and clicking on a flag turns the sort
order.

I added this expression to the mode-line:

        '("D["
          (:eval (my-dired-flags-in-ml-flag-string "-"))
          (:eval (my-dired-flags-in-ml-flag-string "t"))
          (:eval (my-dired-flags-in-ml-flag-string "S"))
          "|"
          (:eval (my-dired-flags-in-ml-flag-string "r"))
          "]")

("D" stands for "dired") whereby

--8<---------------cut here---------------start------------->8---
(defun my-dired-flags-in-ml-flag-string (flag)
  "FLAG is a string of one char."
  `(:propertize ,flag
                face ,(if (pcase flag
                            (`"-" (not (dired-sort-menu-switch-p "[tSXUuc]")))
                            (_ (string-match-p flag dired-actual-switches)))
                          'mode-line-bold nil)
                help-echo ,(pcase flag
                             (`"-" "Sort by name")
                             (`"t" "Sort by time")
                             (`"S" "Sort by size")
                             (`"r" "Reverse sort order"))
                keymap ,(make-mode-line-mouse-map
                         'mouse-2
                         `(lambda (_event)
                            (interactive "e")
                            (pcase ,flag
                              (`"-" (dired-sort-menu-set-switches ""))
                              ((or `"t" `"S")
                               (dired-sort-menu-set-switches ,flag))
                              (`"r"
                              (dired-sort-menu-toggle-reverse)))))))
--8<---------------cut here---------------end--------------->8---


This works well, but every time I click on a flag, I get this message:

| pcase-memoize: equal first branch, yet different

What does that mean?  It's annoying.


Thanks,

Michael.

Re: pcase-memoize: equal first branch, yet different

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> | pcase-memoize: equal first branch, yet different

(when data
        (message "pcase-memoize: equal first branch, yet different"))

in `pcase' seems to be left over from debugging or so.  Maybe we can
comment it out?


Thanks,

Michael.

Re: pcase-memoize: equal first branch, yet different

[Stefan Monnier]

> I get a not so useful message using `pcase' in the following scenario
> using trunk:
[...]
>                          `(lambda (_event)
>                             (interactive "e")
>                             (pcase ,flag
>                               (`"-" (dired-sort-menu-set-switches ""))
>                               ((or `"t" `"S")
>                                (dired-sort-menu-set-switches ,flag))
>                               (`"r"
>                               (dired-sort-menu-toggle-reverse)))))))
[...]
> This works well, but every time I click on a flag, I get this message:
> | pcase-memoize: equal first branch, yet different
> What does that mean?  It's annoying.

The pcase macro's expansion can take a significant amount of time to
generate.  When done during byte-compilation, it's a non-issue, but in
the above example it will be done every time you click.

For this reason, pcase uses a memoization mechanism, to try and reuse
previous expansions.  Sadly, this is unreliable because it is difficult
to figure out when two chunks of code are actually identical and to do
so without incurring undue overheads.

The message is just a warning, so you can ignore it.  But the better
option is to try and fix the underlying problem and make sure pcase is
not expanded repeatedly.

E.g. using lexical-binding:

                   keymap ,(make-mode-line-mouse-map
                            'mouse-2
                            (lambda (_event)
                              (interactive "e")
                              (pcase flag
                                (`"-" (dired-sort-menu-set-switches ""))
                                ((or `"t" `"S")
                                 (dired-sort-menu-set-switches ,flag))
                                (`"r"
                                 (dired-sort-menu-toggle-reverse)))))))

or using CL's lexical-let:

                   keymap ,(make-mode-line-mouse-map
                            'mouse-2
                            (lexical-let ((flag flag))
                              (lambda (_event)
                                (interactive "e")
                                (pcase flag
                                  (`"-" (dired-sort-menu-set-switches ""))
                                  ((or `"t" `"S")
                                   (dired-sort-menu-set-switches ,flag))
                                  (`"r"
                                   (dired-sort-menu-toggle-reverse))))))))

-- Stefan


PS: Why do you use (interactive "e") if you then don't use the
`event' argument.

Re: pcase-memoize: equal first branch, yet different

[Ted Zlatanov]

On Mon, 04 Mar 2013 14:33:41 -0500 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

SM> The pcase macro's expansion can take a significant amount of time to
SM> generate.  When done during byte-compilation, it's a non-issue, but in
SM> the above example it will be done every time you click.

SM> For this reason, pcase uses a memoization mechanism, to try and reuse
SM> previous expansions.  Sadly, this is unreliable because it is difficult
SM> to figure out when two chunks of code are actually identical and to do
SM> so without incurring undue overheads.

Would it make sense to implement some parts of `pcase' at the C level?
As it becomes widely used, byte-compilation delays can also become
significant.

(For the record, I like `pcase' a lot, and really appreciate the
simplicity pattern matching brings to code.)

Ted

Re: pcase-memoize: equal first branch, yet different

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> The message is just a warning, so you can ignore it.  But the better
> option is to try and fix the underlying problem and make sure pcase is
> not expanded repeatedly.

Ah, it's good to know that.

> PS: Why do you use (interactive "e") if you then don't use the
> `event' argument.

I hope this was a rhetorical question ;-) No, there is no reason, thanks
for the hint.


Regards,

Michael.

Re: pcase-memoize: equal first branch, yet different

[Stefan Monnier]

> Would it make sense to implement some parts of `pcase' at the C level?

No, that sounds like a nightmare.

> As it becomes widely used, byte-compilation delays can also become
> significant.

pcase's expansion is not *that* slow.  The problem is when it happens
during evaluation in a loop, where macro expansion takes significantly
more time (and memory) than the naive "try all patterns" would, so it
can slow down evaluation by a large factor (think of
macroexp--expand-all as a typical example: a large portion of the time
is spent matching patterns, which means that it's worthwhile for the
pcase macro to work hard to optimize the resulting code, but if the
macro is re-expanded each time, this effort backfires).

In reality, now that we have eager macroexpansion, the performance
problems should be very rare (i.e. only happen in those rare cases like
the one you posted, where the pcase is inside a chunk of code that's
created at run time) and we could probably get away without memoizing.


        Stefan

Re: pcase-memoize: equal first branch, yet different

[Ted Zlatanov]

On Tue, 05 Mar 2013 10:41:16 -0500 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>> As it becomes widely used, byte-compilation delays can also become
>> significant.
...
SM> In reality, now that we have eager macroexpansion, the performance
SM> problems should be very rare (i.e. only happen in those rare cases like
SM> the one you posted, where the pcase is inside a chunk of code that's
SM> created at run time) and we could probably get away without memoizing.

Cool, thanks for explaining.

Ted