-
Notifications
You must be signed in to change notification settings - Fork 50
[WIP] Add auto-complete suggestions #270
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
[WIP] Add auto-complete suggestions #270
Conversation
3ef84d9 to
a27999c
Compare
a27999c to
3880d2f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This currently works, but there might be some inefficiency here that I'm not spotting. Running purs ide client directly (talking to same purs ide server instance) is instantaneous, while hitting this endpoint takes around 2s, sometimes longer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this could be a POST request with JSON constructed on the client, and passed directly to purs ide client
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's somehow much faster using POST 7496700, but I don't know if there's any security issues with just proxying directly to purs ide client / purs ide server (probably not since I think purs ide client will fail on malformed input?)
138ee85 to
7496700
Compare
thomashoneyman
commented
Feb 8, 2022
@nwolverson -- I'm curious if you have any thoughts on this from the language server side of things; I'm not really sure what is and isn't possible with psc-ide to make this work.
nwolverson
commented
Feb 8, 2022
As per discussion on discord I'd suggest avoiding proxying the purs ide requests directly simply for the reason that it likely allows read/write access to the entire filesystem as the current user, and probably allows more significant DoS potential than a more directed endpoint. However returning an unparsed blob of json to be parsed on the client side seems like a legitimate option if it avoids re-encoding results & can reuse code.
It is surprising that the request would take 2s, it would seem the only extra overhead is an HTTP request, you are spawning the purs binary from the shell as well as the Haskell code. Are you actually looking at the same request in the same workspace? I see groupReexports and maxResults on the left, I'd also prefer to see a better timing (eg just directly using time).
Certainly making the socket request that purs ide client makes directly could be faster but that's something to shave off milliseconds I'd assume.
Happy to discuss the details of the completion results themselves later - but that request looks sane at least
42df7b0 to
0bdd1fd
Compare
This is surprising to me—these are the results from this commit 0bdd1fd (which is back to being a GET request, encoding the JSON on the server)
❯ hyperfine --warmup 1 "curl -Gs --data-urlencode 'q=app' localhost:8081/complete"
Benchmark #1: curl -Gs --data-urlencode 'q=app' localhost:8081/complete
Time (mean ± σ): 173.3 ms ± 10.2 ms [User: 7.3 ms, System: 11.3 ms]
Range (min ... max): 155.2 ms ... 188.8 ms 16 runs
so, pretty snappy, but removing this line
Line 191 in 0bdd1fd
❯ hyperfine --warmup 1 "curl -Gs --data-urlencode 'q=app' localhost:8081/complete"
Benchmark #1: curl -Gs --data-urlencode 'q=app' localhost:8081/complete
Time (mean ± σ): 2.339 s ± 0.118 s [User: 5.0 ms, System: 8.3 ms]
Range (min ... max): 2.209 s ... 2.590 s 10 runs
I added that line because the previous iteration with the POST request was hanging and that seemed to fix it, but I don't fully understand the behavior there.
Other results for completeness :)
From 7496700 POST request (not much difference)
Benchmark #1: curl -s 'http://localhost:8081/complete' -X POST --data-binary '{ "command": "complete", "params": { "filters": [{ "filter": "prefix", "params": { "search": "app" } }] } }' -H 'content-type: application/json'
Time (mean ± σ): 117.7 ms ± 10.5 ms [User: 4.7 ms, System: 7.0 ms]
Range (min ... max): 100.6 ms ... 137.3 ms 21 runs
Running purs ide client directly
Benchmark #1: echo '{ "command": "complete", "params": { "filters": [{ "filter": "prefix", "params": { "search": "app" } }] } }' | purs ide client -p 8082
Time (mean ± σ): 69.8 ms ± 3.2 ms [User: 7.3 ms, System: 16.2 ms]
Range (min ... max): 65.1 ms ... 81.3 ms 33 runs
Uh oh!
There was an error while loading. Please reload this page.
Description of the change
Fixes #32.
Clearly and concisely describe the purpose of the pull request. If this PR relates to an existing issue or change proposal, please link to it. Include any other background context that would help reviewers understand the motivation for this PR.
Checklist: