bug#38561: 27.0.50; Feature request: expose buffer size setting for process filters

bug#38561: 27.0.50; Feature request: expose buffer size setting for process filters

[yyoncho]

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

In GNU Emacs 27.0.50 (build 4, x86_64-pc-linux-gnu, GTK+ Version 3.24.1)
 of 2019-12-10 built on kyoncho-H87-D3H
Repository revision: 8934762bb37273e6606097de92dcc2556456acd2
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12001000
System Description: Linux Mint 19.1

ATM the buffer size is hardcoded to 4k. This will allow configuring
the buffer size based on application needs.

Thanks,
Ivan

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

bug#38561: 27.0.50; Feature request: expose buffer size setting for process filters

[yyoncho]

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

Adding some performance numbers from lsp-mode project.

(with-current-buffer "DemoApplication.java"
  (benchmark-run 10 (lsp-request "workspace/symbol" (list :query "S"))))

;; Emacs from master
=> (4.203047857 0 0.0)

;; tested with emacs compiled with 1mb buffer.
=> (3.132925793 0 0.0)


Thanks,
Ivan

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

bug#38561: 27.0.50; Feature request: expose buffer size setting for process filters

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Tue, 10 Dec 2019 19:25:17 +0200
> 
> Adding some performance numbers from lsp-mode project.
> 
> (with-current-buffer "DemoApplication.java"
>   (benchmark-run 10 (lsp-request "workspace/symbol" (list :query "S"))))
> 
> ;; Emacs from master
> => (4.203047857 0 0.0)
> 
> ;; tested with emacs compiled with 1mb buffer.
> => (3.132925793 0 0.0)

Your wish has been granted, see the new variable
read-process-output-max.

I'd appreciate testing this with your use cases.

bug#38561: 27.0.50; Feature request: expose buffer size setting for process filters

[yyoncho]

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

Hi Eli,

Thank you for taking care of this!

I can confirm that changing the new variable value is yielding the same
results.

I have a question on how am I supposed to use the new setting. Is it
possible to set it per process?

I tried

(with-current-buffer (process-buffer proc)
   (setq-local read-process-output-max (* 1024 1024)))

but it does not work. If it is not possible to use it per process then I
think that I have 2 options:

1. Document it in lsp-mode's readme
2. Override it with a setting in lsp-mode with a bigger value.

Both have clear advantages and disadvantages. Can you advise?

Thanks,
Ivan

On Sat, Dec 21, 2019 at 10:49 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: yyoncho <yyoncho@gmail.com>
> > Date: Tue, 10 Dec 2019 19:25:17 +0200
> >
> > Adding some performance numbers from lsp-mode project.
> >
> > (with-current-buffer "DemoApplication.java"
> >   (benchmark-run 10 (lsp-request "workspace/symbol" (list :query "S"))))
> >
> > ;; Emacs from master
> > => (4.203047857 0 0.0)
> >
> > ;; tested with emacs compiled with 1mb buffer.
> > => (3.132925793 0 0.0)
>
> Your wish has been granted, see the new variable
> read-process-output-max.
>
> I'd appreciate testing this with your use cases.
>

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

bug#38561: 27.0.50; Feature request: expose buffer size setting for process filters

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Sat, 21 Dec 2019 16:43:21 +0200
> Cc: 38561@debbugs.gnu.org
> 
> I can confirm that changing the new variable value is yielding the same results.

Thanks for testing.

> I have a question on how am I supposed to use the new setting. Is it possible to set it per process?

It's a global variable, so you can't.

> 1. Document it in lsp-mode's readme 
> 2. Override it with a setting in lsp-mode with a bigger value.
> 
> Both have clear advantages and disadvantages. Can you advise? 

I'd suggest the former, because if lsp-mode sets the value, it will
affect the entire Emacs session.

If lsp-mode uses accept-process-output, it could bind the variable
inside the form which calls accept-process-output.

bug#38561: 27.0.50; Feature request: expose buffer size setting for process filters

[Eli Zaretskii]

> > 1. Document it in lsp-mode's readme 
> > 2. Override it with a setting in lsp-mode with a bigger value.
> > 
> > Both have clear advantages and disadvantages. Can you advise? 
> 
> I'd suggest the former, because if lsp-mode sets the value, it will
> affect the entire Emacs session.
> 
> If lsp-mode uses accept-process-output, it could bind the variable
> inside the form which calls accept-process-output.

Btw, my assumption was that the value will have to be changed from
default only in very rare situations.  If this is not the case, we
might indeed want that to be a parameter at process creation time (I
don't think we have process-local variables, do we?).

bug#38561: 27.0.50; Feature request: expose buffer size setting for process filters

[yyoncho]

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

I think that it will be fairly common (e. g. everyone using lsp-mode)
will have to use that setting. I am afraid that even if I choose option 1
the distributions like spacemacs/doom will set it globally(because they
are allowed to be opinionated).

When you create a process you provide a bunch of settings(e. g.
:connection-type, :filter, :no-query). Can we make another property for
the read max size?

Thanks,
Ivan

On Sat, Dec 21, 2019 at 8:25 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > > 1. Document it in lsp-mode's readme
> > > 2. Override it with a setting in lsp-mode with a bigger value.
> > >
> > > Both have clear advantages and disadvantages. Can you advise?
> >
> > I'd suggest the former, because if lsp-mode sets the value, it will
> > affect the entire Emacs session.
> >
> > If lsp-mode uses accept-process-output, it could bind the variable
> > inside the form which calls accept-process-output.
>
> Btw, my assumption was that the value will have to be changed from
> default only in very rare situations.  If this is not the case, we
> might indeed want that to be a parameter at process creation time (I
> don't think we have process-local variables, do we?).
>

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

bug#38561: 27.0.50; Feature request: expose buffer size setting for process filters

[Eli Zaretskii]

> From: yyoncho <yyoncho@gmail.com>
> Date: Sat, 21 Dec 2019 21:30:15 +0200
> Cc: 38561@debbugs.gnu.org
> 
> When you create a process you provide a bunch of settings(e. g. 
> :connection-type, :filter, :no-query). Can we make another property for 
> the read max size? 

Thanks what I meant by process parameters.

I'd prefer to wait first and see how many use cases need this setting,
before we decide whether to provide it as a process attribute.

bug#38561: 27.0.50; Feature request: expose buffer size setting for process filters

[yyoncho]

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

Make sense.

(Sorry for misreading your previous mail)


On Sat, Dec 21, 2019 at 9:37 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: yyoncho <yyoncho@gmail.com>
> > Date: Sat, 21 Dec 2019 21:30:15 +0200
> > Cc: 38561@debbugs.gnu.org
> >
> > When you create a process you provide a bunch of settings(e. g.
> > :connection-type, :filter, :no-query). Can we make another property for
> > the read max size?
>
> Thanks what I meant by process parameters.
>
> I'd prefer to wait first and see how many use cases need this setting,
> before we decide whether to provide it as a process attribute.
>

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

bug#38561: 27.0.50; Feature request: expose buffer size setting for process filters

[Dmitry Gutov]

On 21.12.2019 20:25, Eli Zaretskii wrote:
> Btw, my assumption was that the value will have to be changed from
> default only in very rare situations.  If this is not the case, we
> might indeed want that to be a parameter at process creation time (I
> don't think we have process-local variables, do we?).

Could the process use the buffer-local value of this variable?

Or, alternatively, use the value this variable had at the point of 
process's creation. Then the caller could just temporarily bind it.

bug#38561: 27.0.50; Feature request: expose buffer size setting for process filters

[Eli Zaretskii]

> Cc: 38561@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sun, 22 Dec 2019 16:23:13 +0200
> 
> On 21.12.2019 20:25, Eli Zaretskii wrote:
> > Btw, my assumption was that the value will have to be changed from
> > default only in very rare situations.  If this is not the case, we
> > might indeed want that to be a parameter at process creation time (I
> > don't think we have process-local variables, do we?).
> 
> Could the process use the buffer-local value of this variable?

It didn't work for Ivan, I'm not yet sure why.  Feel free to
investigate.

> Or, alternatively, use the value this variable had at the point of 
> process's creation. Then the caller could just temporarily bind it.

That's impossible without saving the value in the process object.