Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance with windows emacs 25.1 #321

Closed
cbsirb opened this issue Aug 1, 2016 · 16 comments
Closed

Performance with windows emacs 25.1 #321

cbsirb opened this issue Aug 1, 2016 · 16 comments

Comments

@cbsirb
Copy link

cbsirb commented Aug 1, 2016

Using irony as the primary completion enigne in company with the latest emacs has a visible delay until showing candidates for a 210 kb .c source file.

After aelp-instrument-package I have this (for emacs 25.1):

  • irony-completion--send-request: 0.5411360333 (time per call)

And for emacs 24.5:

  • irony-completion--send-request: 0.0746239 (time per call)

This is for the same .c file at the same position. If I do a elp-instrument-function for process-send-string, 99% of that time is spend there.

I first thought that this is an issue with process-send-string but I did a small test with emacs:
I created a small program that read from stdin and wrote "SUCCESS" each time it read something and I sent it the same file from emacs with process-send-string, and the results were very different:

For emacs 24.5:

  • process-send-string: 1.0931796 (time per call)

And for emacs 25.1:

  • process-send-string: 0.6128976 (time per call)

I repeated the tests several times, and the results were the same, with small variations (2-5%).

So the issue isn't with process-send-string, but maybe it's something internal to irony-server.

Note: I used emacs 24.5 from msys64 (mingw64 version) and an emacs 25.1 compiled by me with the same flags as emacs 24.5 with mingw64.

@Sarcasm
Copy link
Owner

Sarcasm commented Aug 2, 2016

Hello,

Interesting feedback.
Indeed, playing a bit with completion I can also get irony--send-parse-request to be the "top consumer".

I'm surprised it's not the process-send-string of buffer-substring that causes the issue.

Did you also use format in your test?
Did you test with a little change the middle of the buffer between each test to be more representative?

@cbsirb
Copy link
Author

cbsirb commented Aug 3, 2016

Hello,

I re-did the test and the problem is emacs, not irony. This is the emacs code I used for the test:

(setq proc (start-process "Test" nil "./test.exe"))
(process-send-string proc (format "%d\n%s\n" (point-max) (buffer-substring (point-min) (point-max))))

The test.exe is a simple c++ program:
std::cin >> fileSizeStr;
std::streamsize length = std::stoi(fileSizeStr);
char *content = (char *)malloc(length);
std::cin.read(content, length);

The results were as follows for (elp-instrument-function #'process-send-string)

  • emacs 25.1: 0.510064 seconds/call
  • emacs 24.5: 0.037496 seconds/call

And for a simple C program with fread(..., stdin) instead of std::cin

  • emacs 25.1: 0.547136 seconds/call
  • emacs 24.5: 0.031285 seconds/call

So I should report this to emacs mailing list instead.
You can leave it open, maybe others will have the same problem.

@cbsirb
Copy link
Author

cbsirb commented Aug 3, 2016

The difference is that the original program used a std::cin in a while loop, so it read line-by-line.
Still on that use-case the 25.1 version is twice as fast.

@Sarcasm
Copy link
Owner

Sarcasm commented Aug 3, 2016

Thanks for the feedback, I appreciate.

Please don't hesitate to post a link to the issue here has well, I will follow the thread and it can interest the people seeing this issue too.

@cbsirb
Copy link
Author

cbsirb commented Aug 4, 2016

This is the issue, which apparently isn't one :)
http://lists.gnu.org/archive/html/bug-gnu-emacs/2016-08/msg00048.html

It's fixed by setting w32-pipe-buffer-size to a higher value (I think this was introduced after 24.5 since it doesn't exist there).

I also tested with irony-mode by doing a (setq w32-pipe-buffer-size (point-max)) in irony--send-parse-request and now it works flawlessly.

I don't know what other implications this have for now, but i will set it in my config to a higher value (64 kb) for now.

@Sarcasm
Copy link
Owner

Sarcasm commented Aug 4, 2016

@mambolevis
Copy link

Hi, I changed the default value of the variable in my configuration, just for curiosity/verification, for the new value according to your discussion: (setq w32-pipe-buffer-size 1) and the following error appeared:

irony process stopped!
Error while checking syntax automatically: (file-error "Writing to process" "Bad file descriptor" #<process Irony>)

OS: Windows 10
Emacs: GNU Emacs 25.1.50.1 (x86_64-w64-mingw32) of 2016-07-20
Irony: 0.2.0

@mambolevis
Copy link

I changed the variable to (setq w32-pipe-buffer-size 1)into the function irony--send-parse-request and NO error is generated.
Nonetheless, I noticed a performance problem in my emacs; it runs slowly.

@cbsirb
Copy link
Author

cbsirb commented Aug 4, 2016

It depends on what program is on the other end of the pipe. Some
programs don't like large writes down the pipe. See bug#22344, where
such large writes caused a catastrophic failure. Emacs prefers to err
on the safe side by default, and leave any fine-tuning of the pipe
size to applications that need it.

That's the official response to what implications this change will have.
This is the bug mentioned: http://lists.gnu.org/archive/html/bug-gnu-emacs/2016-01/msg00419.html

I'm busy for the rest of the week, but I'll do some tests Monday and will come back.
I think this should be enabled only for buffers bigger than 100kb.

@Sarcasm
Copy link
Owner

Sarcasm commented Aug 4, 2016

IIUC, the setting is for the whole irony-server process. You should not choose a size based on the current file but more on something that is good enough in the general case for irony-mode.

What surprises me, is that from the documentation and reading of the mailing list posts you linked, I expect the default value for w32-pipe-buffer-size would be the same as it was before, the default pipe size for the platform. I don't understand how it can explain the slowdown. I can understand choosing a bigger value than the default might give better results for big files and we may want to fine-tune this when we starts the irony-server process in irony--start-server-process.

Let me know of your findings, thank you.

@cbsirb
Copy link
Author

cbsirb commented Dec 19, 2016

So, after a long time, I'm back and finally had some time.

For the last 2 months I ran with this in my emacs config:

;; Set the buffer size to 64K on windows (from the original 4K)
(if (>= emacs-major-version 25)
      (setq w32-pipe-buffer-size (* 64 1024)))

The setup:
Windows 10 x64, updated to day
emacs from both emacsbinw64 and the official x64 one
irony built with mingw64 from msys2

There is no performance issue anymore. Maybe you can put this in the FAQ section for the new users.

PS: Not related with this issue, but emacs starts faster and it's more responsive with the official build emacs-25.1-2-x86_64-w64-mingw32.zip and the deps from the same place

@cbsirb cbsirb closed this as completed Dec 19, 2016
Sarcasm added a commit that referenced this issue Dec 19, 2016
This follows the discussion on issue #321:

- #321
@Sarcasm
Copy link
Owner

Sarcasm commented Dec 19, 2016

Hello @Rompy,

First thank you for your feedback, this is valuable.

I added a new variable to irony-mode, which allow to set the value of w32-pipe-buffer-size only for irony-server instead of globally for all Emacs process.

Would you mind giving a try to this setting just to confirm it works for you on Windows?
The best would be to disable your setting (temporarily) for the test and test with the following settings only:

;; Windows performance tweaks
;;
(when (boundp 'w32-pipe-read-delay)
  (setq w32-pipe-read-delay 0))
;; Set the buffer size to 64K on Windows (from the original 4K)
(when (boundp 'w32-pipe-buffer-size)
  (setq irony-server-w32-pipe-buffer-size (* 64 1024)))

C.f. https://github.com/Sarcasm/irony-mode/tree/5f5d40248d3f9700372289bf25791e186665e5e5#windows-considerations

@cbsirb
Copy link
Author

cbsirb commented Dec 19, 2016

So i got the irony.el file from the last commit and set the irony-server-w32-pipe-buffer-size to 64 K and went working as usual. No performance issue so far (and most importantly, no errors).

I also re-tested without setting w32-pipe-buffer-size and irony-server-w32-pipe-buffer-size and whenever i inserted a . or a -> the waiting time is significant.

I think it's safe to use it as a 64 K (or even 32 K). But if you have big C/C++ files, consider increasing this. For example, for a 1 MB C file (>25.000 lines) it's still very slow. I increased irony-server-w32-pipe-buffer-size to 512 K and it was still slow (but acceptable), but I guess that's okay when you have to send 1 MB per keystroke to another process. Maybe it would be a little faster if the irony-server was compiled as a emacs module, but I don't think it's worth the effort.

@Sarcasm
Copy link
Owner

Sarcasm commented Dec 19, 2016

Ok, thank you for the results, much appreciated.

I'm wondering if the size of the pipe matters even for small buffers, for example if there is a lot of completion results.

@cbsirb
Copy link
Author

cbsirb commented Dec 19, 2016

Yes it is. I did a quick test with irony-server-w32-pipe-buffer-size on 0 and included every file in /mingw64/usr/include/c++/6.2.0 (vector, algorithm, etc.) and it's slow.

I logged in irony-completion--request-handler and it sends back between 2600 and 3300 candates.
With 64 K as buffer size it gives the results back instantly.

@Sarcasm
Copy link
Owner

Sarcasm commented Dec 19, 2016

Awesome, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants