fix: disable indexing for unsupported CPUs #3551
Conversation
✅ Deploy Preview for continuedev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@mjkaye - mind running this command on your Debian Bookworm setup and sharing your output? I'd like to confirm on your machine that you're getting the correct output that will allow us to disable indexing. |
|
Having the same crash issue. Running ubuntu 22.04, Here is my cpuinfo: processor : 0 |
Below is the content of /proc/cpuinfo, converted to lower-case. I see that you're checking for AVX support, whereas you should be checking for AVX2 support. As you can see from the output, my CPU supports AVX but not AVX2. This can also be seen in @hex20dec's output. |
Debian Bookworm supports AVX2 and FMA. Instead, this should be tested on a CPU that does not have support for those technologies, such as Sandy Bridge and Ivy Bridge Intel CPUs. |
|
@hex20dec if you change the check to look for avx2 instead of avx does this solve the crash problem? The feedback loop on our side is going to be pretty slow, but if you can try a few different values in the |
|
I've gone ahead and updated the flag to |
|
Ok from my understanding this should work with the avx2 change so I'm going to merge and let it get into our next pre-release. But if anyone notices that it's still broken, please comment in the issue and we'll gladly take another look |
|
Ryzen 7 9800X3D supports both, yet I got this message and now indexing is disabled. I never had any issues with indexing crashing. |
|
Same here unfortunately, after the latest update, Continue decided to stop indexing because of this issue. Also never had any issues with indexing. Is there some way to deactivate this check and revert the decision? |
|
Same here on win11 with an i7-12800h that supports fma, avx, and avx2. Never had an issue with indexing before, but Continue disables indexing now. |
|
I just updated to Version 0.9.251 and after restart I got the notification that indexing was disabled. This is the first time I have had any issue with it. |
|
I just update to latest prerelease version and got the toast about indexing being disabled. First time having an issue, and my CPU supports AVX2 and FMA. |
|
My extension just updated to v0.9.251 prerelease version and the indexing stopped informing my processor is not compatible. I am using AMD Ryzen 7 4700U and according to HwInfo it supports both AVX and FMA. So there must be some defect in the detection mechanism. Attached HwInfo for reference.
|
|
False positive occurred on macOS in MacBook Pro 2019 - this machine's CPU actually supports AVX and FMA instructions. |
|
Just reviewed the code. It seems in Windows platform it is running following command:
This is giving me following response:
The code is then checking if this response has 'avx' & 'fma' words This is causing false positive. |
|
I have been trying to test this PR today, in order to help debug why people are getting the false-positives, on Linux at least. For context, my CPU does not support AVX2 or FMA and experiences the crash reported in #3168 The purpose of this PR, therefore, would be automatically to disable indexing on my machine. Unfortunately the crash, as reported in that issue, occurs before the following is ever run, so I wasn't able to get that far: Line 179 in f92e692 The crash is triggered by: Tracing back through the imports, the originating import statement is: Line 36 in f92e692 I'm not familiar enough with JavaScript/TypeScript to know whether conditional imports are possible. Having said that, I can see that the following would break without that import: Line 399 in f92e692 I'm happy to test any other PRs that might resolve the issue. |
|
I get the the false incompatible error on Windows and my CPU has support for both: |
|
What about
|
|
I have Ryzen 3600, I seriously doubt this is unsupported? |
|
At the risk of just piling on, indexing is disabled for: |
|
same false positive for me on windows, before it was working ok cpu get caption /format:list Caption=Intel64 Family 6 Model 186 Stepping 2 |
|
Hi folks, thank you all for following the toast notification to this PR to share your feedback. It looks like we didn't get this right, so I've disabled the check in this PR: #3714. The PR also implements the dynamic import @mjkaye recommended that should allow users to manually disable indexing in the meantime. We'll ship these updates in the pre-release tonight. We'd still like to get to the bottom of this issue for the users that actually are running unsupported CPUs, so it would be helpful if folks in this thread could share the output of the following respective commands on their machines:
Alternatively, if anyone has a more robust set of commands to check for cc @danielaskdd @amishsthapit @ArulGandhi @AlexJ-StL @mjkaye |
|
Macbook Pro 2019 (CPU: Intel Core i9) |
|
@danielaskdd it looks like your output was same as on my 2019 MBP, e.g. Chatted with @sestinj on this and we are leaning towards just ignoring this check altogether for macOS/Windows users since in the original issues (#940 & #3168) are predominately from Linux users. |
The windows command is useless, it doesnt report cpu capabilities, nor does msinfo32, this would require a custom binary that can check for such capabilities (gcc has such builtin functions for example). |
Have an AMD Ryzen 3600 |
Could well be that support for FMA is being checked but not FMA3 which Ryzen has but not FMA |
|
AMD Ryzen 9 5900X 12-Core Processor |
|
I am taking a look at the code now. With any luck I will have a fix, potentially tonight. Good news or bad news I will update here when I can no longer work on the issue.
|
|
I just tried the pre-released, and the indexing is disabled. I couldn't even add new doc because of it, also I couldn't |
I added some more notes from my testing in #3714 (comment). I hope they're helpful. |
|
@Patrick-Erichsen Sorry for the late update. I worked on this and another project until the sun came up and I passed out. I may be making some headway. It took me a while to figure out where everything was located. I will update here. |
|
I am putting together my findings now. Hopefully I am able to finish it up before I have to deal with some family stuff that will take a few days. With that said, I believe that I have identified some issues and have solved them. I haven't fully tested the out come yet. We'll see how far I get. Shout out to @mjkaye ! You're notes saved me a ton of time and were extremely thoughtful. Lastly, I've never contributed to anything... ever... So, I don't know exactly how to do a pull request and all that, but I will figure it out one way or another. |
|
We will be cutting a release today that temporarily disabled this check. Planning to circle shortly afterwards to incorporate feedback from folks and give this a second try 👍 @AlexJ-StL appreciate all the work here! Check out our contributing docs for some info, and the general GitHub docs for info on things like opening a pull request |
|
I tried v0.9.251 with that feature, but the plugin kept crashing all host. It seems the checking is incorrect in my case, as I am still unable to use it consistently. Please consider creating a patch or adding an option to manually disable indexing. Crashing happens even if i set disable on config |
|
I'm using Windows 11 22H2 (build 22621.1). I know that the CPU I'm using supports However, Continue uses What Users Should DoI modified {
- "disableIndexing": true
+ "disableIndexing": false
}I also modified - "isSupportedLanceDbCpuTarget": false,
+ "isSupportedLanceDbCpuTarget": true,This change may or may not be necessary. I modified const arch2 = import_os3.default.arch();
const platform6 = import_os3.default.platform();
- if (!isSupportedLanceDbCpuTarget(ide)) {
+ if (true) {
globalContext.update("isSupportedLanceDbCpuTarget", true);
return true;
}What Developers Should DoRemove the Continue uses different checks on operating systems other than Windows. I put both of them into GitHub Code Search. I excluded results in Bash since scripts in Bash are not typically run on Windows. Ex grat: if sys.platform in ["linux", "linux2"]:
command = "cat /proc/cpuinfo"
elif sys.platform == "darwin":
command = "/usr/sbin/sysctl -n machdep.cpu.features"
else:
raise UnsupportedPlatformError(f"Unsupported platform: {sys.platform}")if sys.platform in ['linux', 'linux2']:
flags = os.popen('cat /proc/cpuinfo | grep ^flags | head -1').read()
elif sys.platform in ['darwin', ]:
flags = os.popen('sysctl -n machdep.cpu.features machdep.cpu.leaf7_features').read()
else:
raise SystemExit('Windows not supported yet...')Notice how these checks are absent from Windows systems. |
Give me a shout when you have a PR ready to test. |
|
@Windows81 that is a good point. I believe the root of this issue was primarily folks on Linux - going back and looking at previous issues (#3168, #911, #2040), I don't see any reports on Windows or Mac. What do folks think of just updating the CPU check logic to be default true on Windows/Mac, and only run the Linux check ( cc @mjkaye et al for opinions. |
Given that the reports tend to come from Linux users, restricting the check to the Linux platform seems like a reasonable first step. It's logical that users of older CPUs are more likely to be Linux users. Linux performance is still fine on these devices, and we're not forced to upgrade hardware in order to run the latest OS version. I've been running local LLMs for the past year or so, and my system is quite usable. One thing to note is that the command |
Description
Continue currently uses LanceDB for our vector search, but their binaries require that the target CPU has support for
avx2andfma. Previously, if a user's CPU didn't support these features, we would still attempt to invoke the LanceDB binaries, which would then cause the entire extension to crash (see #940 #3168).This PR attempts to detect at runtime if a user's CPU has support for both
avx2andfma, and if not, we disable indexing to prevent subsequent attempts to invoke the LanceDB binary. The first time this occurs we show a toast warning and link to this PR for context.Checklist
Screenshots
Testing instructions
avx2andfmasuch as Debian Bookworm 12.2 (e.g. Crash after installing continue #3168 (comment))