unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#11906: 24.1; completion-at-point failures
@ 2012-07-11  5:54 Leo
  2012-07-12 14:00 ` Stefan Monnier
  0 siblings, 1 reply; 34+ messages in thread
From: Leo @ 2012-07-11  5:54 UTC (permalink / raw)
  To: 11906

Hello Stefan,

Despite these critical points, I have witnessed great improvement over
completions in earlier versions of Emacs. So thanks.

Assume three candidates (ObjC selectors) for completion and
completion-cycle-threshold is 5:

  1. stringWithContentsOfFile:
  2. stringWithContentsOfFile:encoding:error:
  3. stringWithContentsOfFile:usedEncoding:error:

After cycling a few times, I see:
[NSString stringWithContentsOfFile:stringWithContentsOfFile:encoding:error:stringWithContentsOfFile:usedEncodin$

i.e. succeeding completion failed to remove previous one before
inserting its own, it is, in this case, due to : in the completions. But
the problem is general, completion-at-point can be tripped over by chars
in the completion candidates. I can imagine it fails too if completion
contains spaces.

I dug a bit and realised that completion-at-point depended too much on
members in completion-at-point-functions. Those functions are called
multiple times for each trigger of completion. So it can be extremely
slow when the calculation is slow. For example, preparing the completion
table in ObjC could take a few seconds for libclang to analyse the
source and turn the output into something suitable for consumption in
Emacs. It seems completion-at-point should be able to do its entire work
after obtaining once the data from those functions. This would free
users of completion-at-point-functions from worrying about caching.

completion-at-point also invokes those functions in order to decide when
to exit. This causes problems illustrated at the beginning of this
report and, for example, I have also experienced delay in inserting
space, dot, etc following a completion.

Leo





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2012-07-11  5:54 bug#11906: 24.1; completion-at-point failures Leo
@ 2012-07-12 14:00 ` Stefan Monnier
  2013-05-10  6:38   ` Leo Liu
  0 siblings, 1 reply; 34+ messages in thread
From: Stefan Monnier @ 2012-07-12 14:00 UTC (permalink / raw)
  To: Leo; +Cc: 11906

> Assume three candidates (ObjC selectors) for completion and
> completion-cycle-threshold is 5:

>   1. stringWithContentsOfFile:
>   2. stringWithContentsOfFile:encoding:error:
>   3. stringWithContentsOfFile:usedEncoding:error:

> After cycling a few times, I see:
> [NSString
> stringWithContentsOfFile:stringWithContentsOfFile:encoding:error:stringWithContentsOfFile:usedEncodin$

The behavior will surely depend on exactly how you do the above.  So,
could you give more details, such as which modes you're using and which
keys you pressed?

> Emacs.  It seems completion-at-point should be able to do its entire work
> after obtaining once the data from those functions. This would free
> users of completion-at-point-functions from worrying about caching.

Sometimes, you can't get the whole data at once (e.g. completion of
a file-name would have to return all the files in all directories if it
had to be done "a once").

So, this is not an option.  But we could provide a standard
completion-table constructor that provides caching.

> completion-at-point also invokes those functions in order to decide when
> to exit. This causes problems illustrated at the beginning of this
> report and, for example, I have also experienced delay in inserting
> space, dot, etc following a completion.

Can you explain how "this causes problems"?  What makes you think
it's related?


        Stefan





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2012-07-12 14:00 ` Stefan Monnier
@ 2013-05-10  6:38   ` Leo Liu
  2013-05-10 20:36     ` Stefan Monnier
  0 siblings, 1 reply; 34+ messages in thread
From: Leo Liu @ 2013-05-10  6:38 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 11906

On 2012-07-12 22:00 +0800, Stefan Monnier wrote:
>> Emacs.  It seems completion-at-point should be able to do its entire work
>> after obtaining once the data from those functions. This would free
>> users of completion-at-point-functions from worrying about caching.
>
> Sometimes, you can't get the whole data at once (e.g. completion of
> a file-name would have to return all the files in all directories if it
> had to be done "a once").
>
> So, this is not an option.  But we could provide a standard
> completion-table constructor that provides caching.
>
>> completion-at-point also invokes those functions in order to decide when
>> to exit. This causes problems illustrated at the beginning of this
>> report and, for example, I have also experienced delay in inserting
>> space, dot, etc following a completion.
>
> Can you explain how "this causes problems"?  What makes you think
> it's related?

OK, I just hit another performance issue with this repetitive invoking
of completion functions by completion-at-point. To see this issue:

1. emacs -q (choose an emacs that doesn't have the fix in revision 112539)
2. M-x run-octave
3. Type 'uint <TAB>'
4. Type 'history 10'

You should see:

 1040 completion_matches ("uint");

 1041 completion_matches ("uint");

 1042 completion_matches ("uint");

 1043 history 5

So basically computation for the matches against 'uint' has been done
three times. Now when the computation is expensive (such as against the
empty string "") one should observe a terrible delay.

I have to work around this issue in octave by revision 112539.

Leo





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-10  6:38   ` Leo Liu
@ 2013-05-10 20:36     ` Stefan Monnier
  2013-05-11  1:50       ` Leo Liu
  0 siblings, 1 reply; 34+ messages in thread
From: Stefan Monnier @ 2013-05-10 20:36 UTC (permalink / raw)
  To: Leo Liu; +Cc: 11906

>>> completion-at-point also invokes those functions in order to decide when
>>> to exit. This causes problems illustrated at the beginning of this
>>> report and, for example, I have also experienced delay in inserting
>>> space, dot, etc following a completion.
>> Can you explain how "this causes problems"?  What makes you think
>> it's related?
> OK, I just hit another performance issue with this repetitive invoking
> of completion functions by completion-at-point. To see this issue:

> 1. emacs -q (choose an emacs that doesn't have the fix in revision 112539)
> 2. M-x run-octave
> 3. Type 'uint <TAB>'
> 4. Type 'history 10'

I don't understand this recipe: where should I type "history 10"?  right
there after the "uint" text?

> You should see:

>  1040 completion_matches ("uint");

>  1041 completion_matches ("uint");

>  1042 completion_matches ("uint");

>  1043 history 5

Where should I see it?

<fiddling around some more>

Ah, OK.
so I should type RET before "history 10", so history shows me the last
commands run by octave.

> So basically computation for the matches against 'uint' has been done
> three times.

Fine, yes.  There can be various reasons why the completion table is run
several times.  In the present case, 2 is the minimum: once to try and
complete "uint" (which just returns "uint") and once to get the
completion candidates.  Why there's a third call?  I couldn't tell you
off the top of my head.  Maybe it's an inefficiency somewhere.

> Now when the computation is expensive (such as against the
> empty string "") one should observe a terrible delay.

The difference between 2 and 3 calls shouldn't be sufficiently large to
go from "acceptable" to "terrible delay".

> I have to work around this issue in octave by revision 112539.

Using a cache is a good idea: when there's no completion, the completion
code may call the completion-table many more times than just 3 times
(typically, it will call it at least once per completion-style).


        Stefan





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-10 20:36     ` Stefan Monnier
@ 2013-05-11  1:50       ` Leo Liu
  2013-05-11  3:40         ` Stefan Monnier
  0 siblings, 1 reply; 34+ messages in thread
From: Leo Liu @ 2013-05-11  1:50 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 11906

On 2013-05-11 04:36 +0800, Stefan Monnier wrote:
> <fiddling around some more>
>
> Ah, OK.
> so I should type RET before "history 10", so history shows me the last
> commands run by octave.

Sorry, I wasn't clear.

> The difference between 2 and 3 calls shouldn't be sufficiently large to
> go from "acceptable" to "terrible delay".

It is a difference between 1 and 3 calls because a user can also run
octave in terminal and find that how responsive it actually is.

I noticed this long delay when completing empty strings
(octave-completion-at-point used to allow empty strings). Emacs will be
busy for a few seconds (something like 3 ~ 5 seconds in my laptop).
Given how often I use the TAB key, it was terrible experience.

> Using a cache is a good idea: when there's no completion, the completion
> code may call the completion-table many more times than just 3 times
> (typically, it will call it at least once per completion-style).

I just want to point out these problems in the completion-at-point
machinery.

I think if completion-at-point can work well when there is a 2-second
cost in a completion-at-point function, it can provide an excellent
experience.

>         Stefan

Thanks,
Leo





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-11  1:50       ` Leo Liu
@ 2013-05-11  3:40         ` Stefan Monnier
  2013-05-11  4:47           ` Leo Liu
                             ` (2 more replies)
  0 siblings, 3 replies; 34+ messages in thread
From: Stefan Monnier @ 2013-05-11  3:40 UTC (permalink / raw)
  To: Leo Liu; +Cc: 11906

>> The difference between 2 and 3 calls shouldn't be sufficiently large to
>> go from "acceptable" to "terrible delay".
> It is a difference between 1 and 3 calls because a user can also run
> octave in terminal and find that how responsive it actually is.

But the generic completion code can't easily go down to a single call in
the general case.

> I think if completion-at-point can work well when there is a 2-second
> cost in a completion-at-point function, it can provide an excellent
> experience.

Obviously it can, via caching.  Most completion tables which incur
a significant computation code should use caching, but we can't use
caching unconditionally, because it's hard to come up with a general
conditions under which the cache can be reused or needs to be flushed.

As mentioned, we could try and provide a generic completion-table cache
so as to make it easier to write completion tables that have
a significant computation cost.  Patches welcome.


        Stefan





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-11  3:40         ` Stefan Monnier
@ 2013-05-11  4:47           ` Leo Liu
  2013-05-11 14:51             ` Stefan Monnier
  2013-05-11 20:18             ` Andreas Röhler
  2013-05-11 23:11           ` Daimrod
  2013-05-21 23:39           ` Dmitry Gutov
  2 siblings, 2 replies; 34+ messages in thread
From: Leo Liu @ 2013-05-11  4:47 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 11906

On 2013-05-11 11:40 +0800, Stefan Monnier wrote:
> But the generic completion code can't easily go down to a single call
> in the general case.

OK, if that is the case.

I seem to recall that sometimes completion functions are invoked for
getting the start and end positions only. Maybe this is an opportunity
for optimisation.

Leo





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-11  4:47           ` Leo Liu
@ 2013-05-11 14:51             ` Stefan Monnier
  2013-05-13  1:28               ` Leo Liu
  2013-05-11 20:18             ` Andreas Röhler
  1 sibling, 1 reply; 34+ messages in thread
From: Stefan Monnier @ 2013-05-11 14:51 UTC (permalink / raw)
  To: Leo Liu; +Cc: 11906

>> But the generic completion code can't easily go down to a single call
>> in the general case.
> OK, if that is the case.
> I seem to recall that sometimes completion functions are invoked for
> getting the start and end positions only.

The completion-at-point-functions are called repeatedly (once after each
command) to get the start/end, yes.  But not the completion-table.


        Stefan





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-11  4:47           ` Leo Liu
  2013-05-11 14:51             ` Stefan Monnier
@ 2013-05-11 20:18             ` Andreas Röhler
  1 sibling, 0 replies; 34+ messages in thread
From: Andreas Röhler @ 2013-05-11 20:18 UTC (permalink / raw)
  To: 11906

Am 11.05.2013 06:47, schrieb Leo Liu:
> On 2013-05-11 11:40 +0800, Stefan Monnier wrote:
>> But the generic completion code can't easily go down to a single call
>> in the general case.
>
> OK, if that is the case.
>
> I seem to recall that sometimes completion functions are invoked for
> getting the start and end positions only. Maybe this is an opportunity
> for optimisation.
>
> Leo
>
>
>
>

Hi Leo,

from what I've seen some weeks ago, just signaling my interest. Seems a promising point to push things forwards.

Thanks all,

Andreas





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-11  3:40         ` Stefan Monnier
  2013-05-11  4:47           ` Leo Liu
@ 2013-05-11 23:11           ` Daimrod
  2013-05-13 15:28             ` Stefan Monnier
  2013-05-21 23:39           ` Dmitry Gutov
  2 siblings, 1 reply; 34+ messages in thread
From: Daimrod @ 2013-05-11 23:11 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Leo Liu, 11906

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

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

>>> The difference between 2 and 3 calls shouldn't be sufficiently large to
>>> go from "acceptable" to "terrible delay".
>> It is a difference between 1 and 3 calls because a user can also run
>> octave in terminal and find that how responsive it actually is.
>
> But the generic completion code can't easily go down to a single call in
> the general case.
>
>> I think if completion-at-point can work well when there is a 2-second
>> cost in a completion-at-point function, it can provide an excellent
>> experience.
>
> Obviously it can, via caching.  Most completion tables which incur
> a significant computation code should use caching, but we can't use
> caching unconditionally, because it's hard to come up with a general
> conditions under which the cache can be reused or needs to be flushed.
>
> As mentioned, we could try and provide a generic completion-table cache
> so as to make it easier to write completion tables that have
> a significant computation cost.  Patches welcome.

Hello,

Maybe I didn't understand what you mean, but AFAIK it's already
available. You can compute a list of possible completions only once and
then return a completion table (start end COLLECTION) where collection
is a function described here (info "(elisp) Programmed Completion").

-- 
Daimrod/Greg

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-11 14:51             ` Stefan Monnier
@ 2013-05-13  1:28               ` Leo Liu
  2013-05-13 15:27                 ` Stefan Monnier
  0 siblings, 1 reply; 34+ messages in thread
From: Leo Liu @ 2013-05-13  1:28 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 11906

On 2013-05-11 22:51 +0800, Stefan Monnier wrote:
> The completion-at-point-functions are called repeatedly (once after each
> command) to get the start/end, yes.  But not the completion-table.

How about a special variable `completion-requires-no-table' that costly
completion functions can take advantage of the opportunity?

Leo





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-13  1:28               ` Leo Liu
@ 2013-05-13 15:27                 ` Stefan Monnier
  2013-05-14  0:56                   ` Leo Liu
  0 siblings, 1 reply; 34+ messages in thread
From: Stefan Monnier @ 2013-05-13 15:27 UTC (permalink / raw)
  To: Leo Liu; +Cc: 11906

>> The completion-at-point-functions are called repeatedly (once after each
>> command) to get the start/end, yes.  But not the completion-table.
> How about a special variable `completion-requires-no-table' that costly
> completion functions can take advantage of the opportunity?

I do not understand what that variable would do.  Can you give some
more details?


        Stefan





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-11 23:11           ` Daimrod
@ 2013-05-13 15:28             ` Stefan Monnier
  0 siblings, 0 replies; 34+ messages in thread
From: Stefan Monnier @ 2013-05-13 15:28 UTC (permalink / raw)
  To: Daimrod; +Cc: Leo Liu, 11906

>> As mentioned, we could try and provide a generic completion-table cache
>> so as to make it easier to write completion tables that have
>> a significant computation cost.  Patches welcome.

> Maybe I didn't understand what you mean, but AFAIK it's already
> available. You can compute a list of possible completions only once and
> then return a completion table (start end COLLECTION) where collection
> is a function described here (info "(elisp) Programmed Completion").

That's just the easy part of writing a cached completion table.


        Stefan





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-13 15:27                 ` Stefan Monnier
@ 2013-05-14  0:56                   ` Leo Liu
  2013-05-14  2:53                     ` Stefan Monnier
  0 siblings, 1 reply; 34+ messages in thread
From: Leo Liu @ 2013-05-14  0:56 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 11906

On 2013-05-13 23:27 +0800, Stefan Monnier wrote:
> I do not understand what that variable would do.  Can you give some
> more details?

Places that need no completion table should bind
completion-requires-no-table to t before calling the
completion-at-point-functions.

Leo





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-14  0:56                   ` Leo Liu
@ 2013-05-14  2:53                     ` Stefan Monnier
  2013-05-14  3:30                       ` Leo Liu
  0 siblings, 1 reply; 34+ messages in thread
From: Stefan Monnier @ 2013-05-14  2:53 UTC (permalink / raw)
  To: Leo Liu; +Cc: 11906

>> I do not understand what that variable would do.  Can you give some
>> more details?
> Places that need no completion table should bind
> completion-requires-no-table to t before calling the
> completion-at-point-functions.

Makes no sense to me: the completion-table object should always be
"trivial" to build.  At least, that's always been the case so far.


        Stefan





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-14  2:53                     ` Stefan Monnier
@ 2013-05-14  3:30                       ` Leo Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Leo Liu @ 2013-05-14  3:30 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 11906

On 2013-05-14 10:53 +0800, Stefan Monnier wrote:
> Makes no sense to me: the completion-table object should always be
> "trivial" to build.  At least, that's always been the case so far.

OK. In that case I'll leave this off for now until I have more time to
go through minibuffer.el.

Leo





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-11  3:40         ` Stefan Monnier
  2013-05-11  4:47           ` Leo Liu
  2013-05-11 23:11           ` Daimrod
@ 2013-05-21 23:39           ` Dmitry Gutov
  2013-05-22 19:16             ` Stefan Monnier
  2 siblings, 1 reply; 34+ messages in thread
From: Dmitry Gutov @ 2013-05-21 23:39 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Leo Liu, 11906

I've seen this very same problem when writing and using a
completion-at-point function for Ruby, via external live process, so
it's also comparably slow.

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> The difference between 2 and 3 calls shouldn't be sufficiently large to
>>> go from "acceptable" to "terrible delay".
>> It is a difference between 1 and 3 calls because a user can also run
>> octave in terminal and find that how responsive it actually is.
>
> But the generic completion code can't easily go down to a single call in
> the general case.

Why not?

I have a function, called `robe-complete-thing', which is used as a
"dynamic completion table" via `completion-table-dynamic'.

Whenever I press `C-M-i' in a relevant buffer, `robe-complete-thing'
gets called 2 times if the symbol is "complete, but not unique", or 3
times if the symbol is not complete, and the *Completions* buffer should
be displayed.

Whatever code drives this logic, I imagine all places that access the
dynamic completion table do that is specific order. And since they all
look up completions for the same term, can't the first of them save the
lookup result, so that all other places will use the saved value? All
that in the scope of one `complete-symbol' call.

>> I think if completion-at-point can work well when there is a 2-second
>> cost in a completion-at-point function, it can provide an excellent
>> experience.
>
> Obviously it can, via caching.  Most completion tables which incur
> a significant computation code should use caching, but we can't use
> caching unconditionally, because it's hard to come up with a general
> conditions under which the cache can be reused or needs to be flushed.

The one most visible problem case is when the dynamic completion table
is called several times at once for the same term.

Caching is a possible solution, but it doesn't seem to me that caching
anything more than the last request-response pair would be too useful.

> As mentioned, we could try and provide a generic completion-table cache
> so as to make it easier to write completion tables that have
> a significant computation cost.  Patches welcome.

So, suppose we do provide a caching function. Would it cache more than
just one pair? If not, it won't be too hard to do in
`completion-table-dynamic', or in an additional function that would wrap
FUN and then pass it to `completion-table-dynamic'.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-21 23:39           ` Dmitry Gutov
@ 2013-05-22 19:16             ` Stefan Monnier
  2013-12-05  3:23               ` Dmitry Gutov
  0 siblings, 1 reply; 34+ messages in thread
From: Stefan Monnier @ 2013-05-22 19:16 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Leo Liu, 11906

>>>> The difference between 2 and 3 calls shouldn't be sufficiently large to
>>>> go from "acceptable" to "terrible delay".
>>> It is a difference between 1 and 3 calls because a user can also run
>>> octave in terminal and find that how responsive it actually is.
>> But the generic completion code can't easily go down to a single call in
>> the general case.
> Why not?

Because the first call is for try-completion (i.e. "give me the
completion") and the second is for all-completions (i.e. "give me all
matching candidates"), so the info returned by the first call is not
sufficient to avoid the second call.

As you've seen there can be a second call (to try-completion with the
result of the first call to try-completion) to check if the completion
is unique.  Plus another call (to test-completion) to check if the
result of the first try-completion was complete.

> So, suppose we do provide a caching function. Would it cache more than
> just one pair?

Probably, yes.  It would turn test-completion and try-completion into
calls to all-completions and then cache one "arg+result" of
all-completions (this pair would be sufficient to cover all calls to
test/try/all-completion for any argument string which has `arg' as its
prefix).

> If not, it won't be too hard to do in
> `completion-table-dynamic', or in an additional function that would wrap
> FUN and then pass it to `completion-table-dynamic'.

Right, that's the idea.


        Stefan





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-05-22 19:16             ` Stefan Monnier
@ 2013-12-05  3:23               ` Dmitry Gutov
  2013-12-05  4:33                 ` Stefan Monnier
  0 siblings, 1 reply; 34+ messages in thread
From: Dmitry Gutov @ 2013-12-05  3:23 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Leo Liu, 11906

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

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

> Probably, yes.  It would turn test-completion and try-completion into
> calls to all-completions and then cache one "arg+result" of
> all-completions (this pair would be sufficient to cover all calls to
> test/try/all-completion for any argument string which has `arg' as its
> prefix).

How does this patch look?

(The Octave part is 100% untested).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: completion-table-with-cache.diff --]
[-- Type: text/x-diff, Size: 3729 bytes --]

=== modified file 'lisp/minibuffer.el'
--- lisp/minibuffer.el	2013-11-24 14:08:02 +0000
+++ lisp/minibuffer.el	2013-12-05 03:22:09 +0000
@@ -190,6 +190,24 @@
                                (current-buffer)))
         (complete-with-action action (funcall fun string) string pred)))))
 
+(defun completion-table-with-cache (fun &optional ignore-case)
+  "Create dynamic completion table from FUN, with cache.
+This wraps `completion-table-dynamic', but saves the last
+argument-result pair from FUN, so that several lookups with the
+same argument (or with an argument that starts with the first one)
+only need to call FUN once.  Most useful when FUN performs a relatively
+slow operation, such as calling an external process (see Bug#11906).
+When IGNORE-CASE is non-nil, FUN is expected to be case-insensitive."
+  (let* (last-arg last-result
+         (new-fun
+          (lambda (arg)
+            (if (and last-arg (string-prefix-p last-arg arg ignore-case))
+                last-result
+              (prog1
+                  (setq last-result (funcall fun arg))
+                (setq last-arg arg))))))
+    (completion-table-dynamic new-fun)))
+
 (defmacro lazy-completion-table (var fun)
   "Initialize variable VAR as a lazy completion table.
 If the completion table VAR is used for the first time (e.g., by passing VAR

=== modified file 'lisp/progmodes/octave.el'
--- lisp/progmodes/octave.el	2013-12-02 07:13:01 +0000
+++ lisp/progmodes/octave.el	2013-12-05 03:15:06 +0000
@@ -838,21 +838,13 @@
     ;; `comint-history-isearch-backward-regexp'.  Bug#14433.
     (comint-send-string proc "\n")))
 
-(defvar inferior-octave-completion-table
-  ;;
-  ;; Use cache to avoid repetitive computation of completions due to
-  ;; bug#11906 - http://debbugs.gnu.org/11906 - which may cause
-  ;; noticeable delay.  CACHE: (CMD . VALUE).
-  (let ((cache))
-    (completion-table-dynamic
-     (lambda (command)
-       (unless (equal (car cache) command)
-         (inferior-octave-send-list-and-digest
-          (list (format "completion_matches ('%s');\n" command)))
-         (setq cache (cons command
-                           (delete-consecutive-dups
-                            (sort inferior-octave-output-list 'string-lessp)))))
-       (cdr cache)))))
+(defun inferior-octave-completion-table ()
+  (completion-table-with-cache
+   (lambda (command)
+     (inferior-octave-send-list-and-digest
+      (list (format "completion_matches ('%s');\n" command)))
+     (delete-consecutive-dups
+      (sort inferior-octave-output-list 'string-lessp)))))
 
 (defun inferior-octave-completion-at-point ()
   "Return the data to complete the Octave symbol at point."
@@ -864,7 +856,7 @@
           (end (point)))
       (when (and beg (> end beg))
         (list beg end (completion-table-in-turn
-                       inferior-octave-completion-table
+                       (inferior-octave-completion-table)
                        'comint-completion-file-name-table))))))
 
 (define-obsolete-function-alias 'inferior-octave-complete
@@ -1022,7 +1014,7 @@
     (completing-read
      (format (if def "Function (default %s): "
                "Function: ") def)
-     inferior-octave-completion-table
+     (inferior-octave-completion-table)
      nil nil nil nil def)))
 
 (defun octave-goto-function-definition (fn)
@@ -1406,7 +1398,7 @@
                         (setq end (point))))
     (when (> end beg)
       (list beg end (or (and (inferior-octave-process-live-p)
-                             inferior-octave-completion-table)
+                             (inferior-octave-completion-table))
                         octave-reserved-words)))))
 
 (define-obsolete-function-alias 'octave-complete-symbol


^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-05  3:23               ` Dmitry Gutov
@ 2013-12-05  4:33                 ` Stefan Monnier
  2013-12-06  1:02                   ` Dmitry Gutov
  0 siblings, 1 reply; 34+ messages in thread
From: Stefan Monnier @ 2013-12-05  4:33 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Leo Liu, 11906

>> Probably, yes.  It would turn test-completion and try-completion into
>> calls to all-completions and then cache one "arg+result" of
>> all-completions (this pair would be sufficient to cover all calls to
>> test/try/all-completion for any argument string which has `arg' as its
>> prefix).
> How does this patch look?
> (The Octave part is 100% untested).

Looks OK, thank you.  We may want to extend it at some point with
a predicate that can test if the cache is stale, but for now it's
probably good enough.


        Stefan





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-05  4:33                 ` Stefan Monnier
@ 2013-12-06  1:02                   ` Dmitry Gutov
  2013-12-06  4:00                     ` Leo Liu
  0 siblings, 1 reply; 34+ messages in thread
From: Dmitry Gutov @ 2013-12-06  1:02 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Leo Liu, 11906

On 05.12.2013 06:33, Stefan Monnier wrote:
> Looks OK, thank you.

Installed, seems to work fine.

 > We may want to extend it at some point with
> a predicate that can test if the cache is stale, but for now it's
> probably good enough.

Probably, but for its primary usage (amortizing the 2-3 lookups 
`completion-at-point' does) even the `string-prefix-p' check is 
redundant, just as long as we create a new completion-table each time 
our completion-at-point function is called, and not cache it in a var.


Leo, do you consider this bug fixed now, or would you like to provide a 
reproduction scenario for the ObjC selector completion cycling problem, 
mentioned in the initial report?





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-06  1:02                   ` Dmitry Gutov
@ 2013-12-06  4:00                     ` Leo Liu
  2013-12-06  4:32                       ` Dmitry Gutov
  0 siblings, 1 reply; 34+ messages in thread
From: Leo Liu @ 2013-12-06  4:00 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 11906

On 2013-12-06 09:02 +0800, Dmitry Gutov wrote:
> Leo, do you consider this bug fixed now, or would you like to provide
> a reproduction scenario for the ObjC selector completion cycling
> problem, mentioned in the initial report?

I see the solution has gone down in a different route.

My intention was to fix/avoid inefficiency in minibuffer.el in the first
place.

The code in minibuffer.el knows perfectly well when it doesn't need a
completion table and should provide a way to notify
completion-at-point-functions so that they can simplify ignore such
computation.

Leo





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-06  4:00                     ` Leo Liu
@ 2013-12-06  4:32                       ` Dmitry Gutov
  2013-12-06  5:36                         ` Leo Liu
  0 siblings, 1 reply; 34+ messages in thread
From: Dmitry Gutov @ 2013-12-06  4:32 UTC (permalink / raw)
  To: Leo Liu; +Cc: 11906

On 06.12.2013 06:00, Leo Liu wrote:
> The code in minibuffer.el knows perfectly well when it doesn't need a
> completion table and should provide a way to notify
> completion-at-point-functions so that they can simplify ignore such
> computation.

I don't understand what you mean by "doesn't need a completion table". 
Could you give an example?

If some function doesn't need it, why does it use it? There should be no 
need to notify anything.

Or do you mean that instead of the "full" table, it just requires one 
match (where `try-completion' is used)? It may reduce the amount of 
computation performed by the backend function, but not always by much.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-06  4:32                       ` Dmitry Gutov
@ 2013-12-06  5:36                         ` Leo Liu
  2013-12-06 13:15                           ` Dmitry Gutov
  0 siblings, 1 reply; 34+ messages in thread
From: Leo Liu @ 2013-12-06  5:36 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 11906

On 2013-12-06 12:32 +0800, Dmitry Gutov wrote:
> I don't understand what you mean by "doesn't need a completion table".
> Could you give an example?

See completion-at-point:

      (let ((newstart (car-safe (funcall hookfun))))
              (and newstart (= newstart start)))

so basically every command following completion-at-point calls HOOKFUN
to check if start matches, in this case it doesn't need the completion
table. Hopefully There will be more places to optimise if studying
minibuffer.el in more details.

Leo





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-06  5:36                         ` Leo Liu
@ 2013-12-06 13:15                           ` Dmitry Gutov
  2013-12-06 14:04                             ` Leo Liu
  0 siblings, 1 reply; 34+ messages in thread
From: Dmitry Gutov @ 2013-12-06 13:15 UTC (permalink / raw)
  To: Leo Liu; +Cc: 11906

On 06.12.2013 07:36, Leo Liu wrote:
> See completion-at-point:
>
>        (let ((newstart (car-safe (funcall hookfun))))
>                (and newstart (= newstart start)))
>
> so basically every command following completion-at-point calls HOOKFUN
> to check if start matches, in this case it doesn't need the completion
> table.

But that function is fast! Compared to doing the actual completion, the 
time it takes to `(funcall hookfun)' should be negligible:

ELISP> (js2-time (setq ocap (with-current-buffer "*Inferior Octave*" 
(octave-completion-at-point))))
0.0
ELISP> (js2-time (with-current-buffer "*Inferior Octave*" (funcall (nth 
2 ocap) "a" nil t)))
0.0055






^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-06 13:15                           ` Dmitry Gutov
@ 2013-12-06 14:04                             ` Leo Liu
  2013-12-06 17:35                               ` Stefan Monnier
                                                 ` (2 more replies)
  0 siblings, 3 replies; 34+ messages in thread
From: Leo Liu @ 2013-12-06 14:04 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 11906

On 2013-12-06 21:15 +0800, Dmitry Gutov wrote:
> But that function is fast! Compared to doing the actual completion,
> the time it takes to `(funcall hookfun)' should be negligible:
>
> ELISP> (js2-time (setq ocap (with-current-buffer "*Inferior Octave*"
> (octave-completion-at-point))))
> 0.0
> ELISP> (js2-time (with-current-buffer "*Inferior Octave*" (funcall
> (nth 2 ocap) "a" nil t)))
> 0.0055

I see. Let's consider the performance issue fixed for now.

Another issue as mentioned in the report is when you complete, for
example, 'abc' to 'aa bb cc' (or whatever strange chars are in the
completion candidate) and the completion function fails to go back to
the start.

Can this be improved? On a case to base basis the completion function
could use a marker so that it can subsequently find the starting point.
But can minibuffer.el handle this so that all completion function don't
need to worry about this?

Also instead of calling completion function to check if start has
changed to decide to exit completion-in-region-mode, how about let any
char insertion or deletion exit the mode instead?

Leo





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-06 14:04                             ` Leo Liu
@ 2013-12-06 17:35                               ` Stefan Monnier
  2013-12-07  2:05                                 ` Leo Liu
  2013-12-06 17:36                               ` Stefan Monnier
  2013-12-07  2:02                               ` Dmitry Gutov
  2 siblings, 1 reply; 34+ messages in thread
From: Stefan Monnier @ 2013-12-06 17:35 UTC (permalink / raw)
  To: Leo Liu; +Cc: 11906, Dmitry Gutov

> Another issue as mentioned in the report is when you complete, for
> example, 'abc' to 'aa bb cc' (or whatever strange chars are in the
> completion candidate) and the completion function fails to go back to
> the start.

This would be a bug (probably in the completion-at-point-function).

> Can this be improved? On a case to base basis the completion function
> could use a marker so that it can subsequently find the starting point.

That should "never" be necessary.


        Stefan





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-06 14:04                             ` Leo Liu
  2013-12-06 17:35                               ` Stefan Monnier
@ 2013-12-06 17:36                               ` Stefan Monnier
  2013-12-07  2:02                               ` Dmitry Gutov
  2 siblings, 0 replies; 34+ messages in thread
From: Stefan Monnier @ 2013-12-06 17:36 UTC (permalink / raw)
  To: Leo Liu; +Cc: 11906, Dmitry Gutov

> Also instead of calling completion function to check if start has
> changed to decide to exit completion-in-region-mode, how about let any
> char insertion or deletion exit the mode instead?

There's no technical reason not to, but... why would we want to do that?


        Stefan





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-06 14:04                             ` Leo Liu
  2013-12-06 17:35                               ` Stefan Monnier
  2013-12-06 17:36                               ` Stefan Monnier
@ 2013-12-07  2:02                               ` Dmitry Gutov
  2013-12-07  2:40                                 ` Leo Liu
  2 siblings, 1 reply; 34+ messages in thread
From: Dmitry Gutov @ 2013-12-07  2:02 UTC (permalink / raw)
  To: Leo Liu; +Cc: 11906

On 06.12.2013 16:04, Leo Liu wrote:
> Another issue as mentioned in the report is when you complete, for
> example, 'abc' to 'aa bb cc' (or whatever strange chars are in the
> completion candidate) and the completion function fails to go back to
> the start.

It seems to me that `completion-at-point' isn't a good facility to 
complete space-separated lists of words or symbols (unlike, say, 
hippie-expand).

Suppose it works, and you have candidates: "aa bb cc", "aa bd ee", 
"aabbc ef". You type "aa", press C-M-i, it completes to the common 
prefix: "aa b". Even if `completion-at-point' still remembers where the 
candidate started, what if you exit `completion-in-region-mode' via, 
say, cursor, movement, and then go back to after "aa b". When you press 
C-M-i again, what completion candidates would you expect to see? Not "aa 
bb cc" and "aa bd ee", right?

Note that this can be fixed in specific completion-at-point functions. 
For example, Objective-C completion can look at the context, or maybe 
just always treat semicolons as symbol constituents (I don't really know 
the syntax).

> Also instead of calling completion function to check if start has
> changed to decide to exit completion-in-region-mode, how about let any
> char insertion or deletion exit the mode instead?

Could be good for some cases and users, but this prohibits the user from 
looking at the completions buffer and typing one of the candidates, 
manually (maybe a part of it, until it's unique).

Hiding the completions buffer right after one character is typed can 
make it less useful.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-06 17:35                               ` Stefan Monnier
@ 2013-12-07  2:05                                 ` Leo Liu
  2013-12-07 22:45                                   ` Stefan Monnier
  0 siblings, 1 reply; 34+ messages in thread
From: Leo Liu @ 2013-12-07  2:05 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 11906, Dmitry Gutov

On 2013-12-07 01:35 +0800, Stefan Monnier wrote:
[snipped 5 lines]
> This would be a bug (probably in the completion-at-point-function).
[snipped 4 lines]
> That should "never" be necessary.

On 2013-12-07 01:36 +0800, Stefan Monnier wrote:
[snipped 3 lines]
> There's no technical reason not to, but... why would we want to do that?
[snipped 3 lines]

For this case:

--8<---------------cut here---------------start------------->8---
Assume three candidates (ObjC selectors) for completion and
completion-cycle-threshold is 5:

  1. stringWithContentsOfFile:
  2. stringWithContentsOfFile:encoding:error:
  3. stringWithContentsOfFile:usedEncoding:error:

After cycling a few times, I see:
[NSString stringWithContentsOfFile:stringWithContentsOfFile:encoding:error:stringWithContentsOfFile:usedEncodin$
--8<---------------cut here---------------end--------------->8---

So indeed it is the capf's failure to go back to the proper starting
point after the insertion of the first completion candidate. But the
capf could have no idea what chars the completion candidates can be made
of. In the example it is : that capf needs to handle. sometimes it is
SPC or [ or { or whatever. How can this be handled reliably? Thus my
proposal to use marker. i.e. in the beginning of a completion session
set a start marker.

In general the idea is to make the completion machinery in minibuffer.el
only have to ask capf for information once for each completion session.

Leo





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-07  2:02                               ` Dmitry Gutov
@ 2013-12-07  2:40                                 ` Leo Liu
  2013-12-07 16:13                                   ` Dmitry Gutov
  0 siblings, 1 reply; 34+ messages in thread
From: Leo Liu @ 2013-12-07  2:40 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 11906

On 2013-12-07 10:02 +0800, Dmitry Gutov wrote:
> It seems to me that `completion-at-point' isn't a good facility to
> complete space-separated lists of words or symbols (unlike, say,
> hippie-expand).
>
> Suppose it works, and you have candidates: "aa bb cc", "aa bd ee",
> "aabbc ef". You type "aa", press C-M-i, it completes to the common
> prefix: "aa b". Even if `completion-at-point' still remembers where
> the candidate started, what if you exit `completion-in-region-mode'
> via, say, cursor, movement, and then go back to after "aa b". When you
> press C-M-i again, what completion candidates would you expect to see?
> Not "aa bb cc" and "aa bd ee", right?
>
[snipped 9 lines]
> Could be good for some cases and users, but this prohibits the user
> from looking at the completions buffer and typing one of the
> candidates, manually (maybe a part of it, until it's unique).
>
> Hiding the completions buffer right after one character is typed can
> make it less useful.

These are good points. I think we need a new way for completion. Thanks
for your work and feel free to close this bug.

Leo





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-07  2:40                                 ` Leo Liu
@ 2013-12-07 16:13                                   ` Dmitry Gutov
  2013-12-09  2:27                                     ` Leo Liu
  0 siblings, 1 reply; 34+ messages in thread
From: Dmitry Gutov @ 2013-12-07 16:13 UTC (permalink / raw)
  To: Leo Liu; +Cc: 11906-done

On 07.12.2013 04:40, Leo Liu wrote:
> [snipped 9 lines]

I think you skipped the most important practical point - that your 
initial Object-C example can be made to work properly using the current 
completion-at-point-functions interface.

> These are good points. I think we need a new way for completion. Thanks
> for your work and feel free to close this bug.

Sure, no problem.






^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-07  2:05                                 ` Leo Liu
@ 2013-12-07 22:45                                   ` Stefan Monnier
  0 siblings, 0 replies; 34+ messages in thread
From: Stefan Monnier @ 2013-12-07 22:45 UTC (permalink / raw)
  To: Leo Liu; +Cc: 11906, Dmitry Gutov

> --8<---------------cut here---------------start------------->8---
> Assume three candidates (ObjC selectors) for completion and
> completion-cycle-threshold is 5:

>   1. stringWithContentsOfFile:
>   2. stringWithContentsOfFile:encoding:error:
>   3. stringWithContentsOfFile:usedEncoding:error:

> After cycling a few times, I see:
> [NSString
> stringWithContentsOfFile:stringWithContentsOfFile:encoding:error:stringWithContentsOfFile:usedEncodin$
> --8<---------------cut here---------------end--------------->8---

Ah, you're talking cycling, not just completion.  Yes, cycling needs
special treatment since a full candidate is inserted, after which it may
very well be that point is not inside the completion field any more.

This said, minibuffer.el already has some special treatment for it, but
I guess it doesn't cut it in your case.  Can you show a concrete test
case (ideally starting from "emacs -Q")?


        Stefan





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#11906: 24.1; completion-at-point failures
  2013-12-07 16:13                                   ` Dmitry Gutov
@ 2013-12-09  2:27                                     ` Leo Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Leo Liu @ 2013-12-09  2:27 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 11906

On 2013-12-08 00:13 +0800, Dmitry Gutov wrote:
> I think you skipped the most important practical point - that your
> initial Object-C example can be made to work properly using the
> current completion-at-point-functions interface.

Not really. I have fixed the specific capf long ago and I knew it could
be fixed in that case. And my objective C completer is bit-rotting so I
cannot test it. My bug report was hoping to trigger a change in emacs's
completion facility.

Thanks for the fix,
Leo





^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2013-12-09  2:27 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-11  5:54 bug#11906: 24.1; completion-at-point failures Leo
2012-07-12 14:00 ` Stefan Monnier
2013-05-10  6:38   ` Leo Liu
2013-05-10 20:36     ` Stefan Monnier
2013-05-11  1:50       ` Leo Liu
2013-05-11  3:40         ` Stefan Monnier
2013-05-11  4:47           ` Leo Liu
2013-05-11 14:51             ` Stefan Monnier
2013-05-13  1:28               ` Leo Liu
2013-05-13 15:27                 ` Stefan Monnier
2013-05-14  0:56                   ` Leo Liu
2013-05-14  2:53                     ` Stefan Monnier
2013-05-14  3:30                       ` Leo Liu
2013-05-11 20:18             ` Andreas Röhler
2013-05-11 23:11           ` Daimrod
2013-05-13 15:28             ` Stefan Monnier
2013-05-21 23:39           ` Dmitry Gutov
2013-05-22 19:16             ` Stefan Monnier
2013-12-05  3:23               ` Dmitry Gutov
2013-12-05  4:33                 ` Stefan Monnier
2013-12-06  1:02                   ` Dmitry Gutov
2013-12-06  4:00                     ` Leo Liu
2013-12-06  4:32                       ` Dmitry Gutov
2013-12-06  5:36                         ` Leo Liu
2013-12-06 13:15                           ` Dmitry Gutov
2013-12-06 14:04                             ` Leo Liu
2013-12-06 17:35                               ` Stefan Monnier
2013-12-07  2:05                                 ` Leo Liu
2013-12-07 22:45                                   ` Stefan Monnier
2013-12-06 17:36                               ` Stefan Monnier
2013-12-07  2:02                               ` Dmitry Gutov
2013-12-07  2:40                                 ` Leo Liu
2013-12-07 16:13                                   ` Dmitry Gutov
2013-12-09  2:27                                     ` Leo Liu

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).