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

Getting aborted when running CommnadT in lua version #399

Closed
mohammad5305 opened this issue Aug 29, 2022 · 7 comments
Closed

Getting aborted when running CommnadT in lua version #399

mohammad5305 opened this issue Aug 29, 2022 · 7 comments

Comments

@mohammad5305
Copy link

mohammad5305 commented Aug 29, 2022

image
image

also I tried with neovim 0.7 and setting ARCHFLAGS to i386 in compile but getting same result
os: voidlinux i386
neovim: 0.8
package manager: packer

@wincent
Copy link
Owner

wincent commented Aug 29, 2022

Thanks for the report @mohammad5305. I have a couple of ideas for how we might gather more information. One is to make a debug build, which you can do like this:

cd ~/.local/share/nvim/site/pack/packer/start/command-t/lua/wincent/commandt/lib
DEBUG=1 make

That will create a build with some assert() calls enabled that might reveal a problem, and it will also write to a log file commandt-debug.log in the current directory (it doesn't actually log much at this time, so you might not see anything in there...).

The other thing you could do is run the test suite:

cd ~/.local/share/nvim/site/pack/packer/start/command-t
bin/test

One final question: what are you doing before and when the abort happens?

@mohammad5305
Copy link
Author

test are passing successfully expect one

       SKIP  doesn't show a dotfile just because there was a match at index 0 (fix: see ed01bc6)

and here is log file

and nothing I just open neovim normally without anything else after that when I execute CommandT or other finders I get aborted and i'm back to shell

@wincent
Copy link
Owner

wincent commented Aug 29, 2022

@mohammad5305: My guess is that because you're on a 32-bit arch our generous allocations of virtual memory are eventually causing the abort (ie. because we're running out of address space).

You could try this patch (which cuts the max file count by 4 and cuts the buffer size by 4 too) and run make again. Potentially even drop the numbers lower if that doesn't work. If we can find "the right" tuning for the parameters, we can bake this into the Makefile so that it will use appropriate limits on 32-bit and 64-bit machines.

diff --git a/lua/wincent/commandt/lib/find.c b/lua/wincent/commandt/lib/find.c
index 2d85e88..f87a76f 100644
--- a/lua/wincent/commandt/lib/find.c
+++ b/lua/wincent/commandt/lib/find.c
@@ -21,8 +21,8 @@
 #include "xstrdup.h" /* for xstrdup() */
 
 // TODO: share these with scanner.c
-static long MAX_FILES = 134217728; // 128 M candidates.
-static size_t buffer_size = 137438953472; // 128 GB.
+static long MAX_FILES = 33554432; // 32 M candidates.
+static size_t buffer_size = 34359738368; // 32 GB.
 static const char *current_directory = ".";
 
 find_result_t *commandt_find(const char *directory) {
diff --git a/lua/wincent/commandt/lib/scanner.c b/lua/wincent/commandt/lib/scanner.c
index edea023..7ded5c9 100644
--- a/lua/wincent/commandt/lib/scanner.c
+++ b/lua/wincent/commandt/lib/scanner.c
@@ -19,9 +19,9 @@
 // TODO: make this capable of producing asynchronously?
 
 // TODO make this configurable
-static long MAX_FILES = 134217728; // 128 M candidates.
-
-static size_t buffer_size = 137438953472; // 128 GB.
+static long MAX_FILES = 33554432; // 32 M candidates.
+                                  //
+static size_t buffer_size = 34359738368; // 32 GB.
 
 scanner_t *scanner_new_copy(const char **candidates, unsigned count) {
     scanner_t *scanner = xcalloc(1, sizeof(scanner_t));

@mohammad5305
Copy link
Author

yeah the problem fixed thanks

wincent added a commit that referenced this issue Aug 29, 2022
To deal with the aborts reported in:

    #399

My theory was that on a 32-bit machine an allocation was failing due to
the rather generous sizes involved, given that the DEBUG build showed
the tests passing, and some allocations were recorded in the
`commandt-debug.log`

We tried with smaller sizes (1/4 of the previous sizes) and the aborts
went away. These sizes are still ridiculously large. The smaller sizes
correspond up to 32 million candidate strings (the largest projects I've
ever worked with or even heard of had anywhere between 1.5 M and 4 M
strings; I expect that anybody working on anything larger than that is
very, very likely running on a 64-bit architecture), and a string
storage slab of 32 G (the way the math works out here is if you had 32 M
strings of length 1K you'd fill up all the space).

So, in this commit, we do some basic autodetection based on `getconf
LONG_BIT` (which is POSIX, so should be relatively portable, even if it
isn't super informative) and use that to decide how big the numbers
should be.

We may still want to make this overridable by users, or otherwise more
sophisticated, but until I see a demand for that, I'll hold off.

The `compile_flags.txt` file is so that `clangd` can know what the
preprocessor options are going to resemble (obviously, these are
hard-coded, so really are just intended to keep the LSP from complaining
about undeclared identifiers).
@wincent
Copy link
Owner

wincent commented Aug 29, 2022

Thanks for confirming, @mohammad5305.

I pushed a tweak in 7b47958 that should make this automatic — based on the output of getconf LONG_BIT (which should be 32 or 64, typically), it will pick suitable values for the memory allocations. 🤞

@mohammad5305
Copy link
Author

image
looks like the commit have some problems

@wincent
Copy link
Owner

wincent commented Aug 30, 2022

Thanks for letting me know, @mohammad5305.

  • The good news: this shows that the build is correctly identifying your system as 32-bit.
  • The bad news: I had a copy-paste error in the 32-bit code where I forgot to include the -D prefix in the relevant parts of the Makefile.

Fixed that in 6c8e2a3.

Padmamanickam added a commit to Padmamanickam/command-t that referenced this issue Feb 6, 2023
To deal with the aborts reported in:

    wincent/command-t#399

My theory was that on a 32-bit machine an allocation was failing due to
the rather generous sizes involved, given that the DEBUG build showed
the tests passing, and some allocations were recorded in the
`commandt-debug.log`

We tried with smaller sizes (1/4 of the previous sizes) and the aborts
went away. These sizes are still ridiculously large. The smaller sizes
correspond up to 32 million candidate strings (the largest projects I've
ever worked with or even heard of had anywhere between 1.5 M and 4 M
strings; I expect that anybody working on anything larger than that is
very, very likely running on a 64-bit architecture), and a string
storage slab of 32 G (the way the math works out here is if you had 32 M
strings of length 1K you'd fill up all the space).

So, in this commit, we do some basic autodetection based on `getconf
LONG_BIT` (which is POSIX, so should be relatively portable, even if it
isn't super informative) and use that to decide how big the numbers
should be.

We may still want to make this overridable by users, or otherwise more
sophisticated, but until I see a demand for that, I'll hold off.

The `compile_flags.txt` file is so that `clangd` can know what the
preprocessor options are going to resemble (obviously, these are
hard-coded, so really are just intended to keep the LSP from complaining
about undeclared identifiers).
Padmamanickam added a commit to Padmamanickam/command-t that referenced this issue Feb 6, 2023
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

2 participants