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

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

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

yyoncho
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
Reply | Threaded
Open this post in threaded view
|

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

yyoncho
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
Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> From: yyoncho <[hidden email]>
> 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.



Reply | Threaded
Open this post in threaded view
|

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

yyoncho
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 <[hidden email]> wrote:
> From: yyoncho <[hidden email]>
> 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.
Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> From: yyoncho <[hidden email]>
> Date: Sat, 21 Dec 2019 16:43:21 +0200
> Cc: [hidden email]
>
> 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.



Reply | Threaded
Open this post in threaded view
|

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?).



Reply | Threaded
Open this post in threaded view
|

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

yyoncho
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 <[hidden email]> 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?).
Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> From: yyoncho <[hidden email]>
> Date: Sat, 21 Dec 2019 21:30:15 +0200
> Cc: [hidden email]
>
> 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.



Reply | Threaded
Open this post in threaded view
|

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

yyoncho
Make sense. 

(Sorry for misreading your previous mail)


On Sat, Dec 21, 2019 at 9:37 PM Eli Zaretskii <[hidden email]> wrote:
> From: yyoncho <[hidden email]>
> Date: Sat, 21 Dec 2019 21:30:15 +0200
> Cc: [hidden email]
>
> 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.
Reply | Threaded
Open this post in threaded view
|

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

Dmitry Gutov
In reply to this post by Eli Zaretskii
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.



Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> Cc: [hidden email]
> From: Dmitry Gutov <[hidden email]>
> 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.