I3oris / ic

REPL Interface wrapping Crystal Interpreter (crystal-i).
MIT License
49 stars 2 forks source link

Load crystal stdlib in background during REPL startup #7

Closed Vici37 closed 2 years ago

Vici37 commented 2 years ago

Gist is that the stdlib could be more efficiently loaded in the background while the user is entering in their first line :) This cuts REPL startup time from ~1/2 second to instantaneously, and everything loads correctly.

I tested adding a sleep 30 in the spawn below to simulate user input before everything was loaded, and ended up seeing a much shorter list in the tab complete of what I could do (as expected). Waiting out the 30 seconds had everything load in the same session and tab complete "fixed" itself.

I3oris commented 2 years ago

Hello Vici37, thank you for coming with this.

The prompt starts instantaneously but then, the user get stuck (with no echo of its input) during the loading of the prelude. Adding the sleep 30 make the editing loop work correctly during these 30 second, but the prelude is only loaded after the delay and block the user until it finish. This end up to be just equivalent to display the prompt before loading prelude :/.

You can better see what happen (or when) with this:

spawn do
  puts "load prelude"
  load_prelude
  puts "prelude loaded!"
end 

I have already thought to this problem before, but didn't found a easy solution. The main problem is that load_prelude will always be executed at once, with no way to execute code between (output echo, tabulation...). I think it's due to concurrency model.

Anyway, maybe just display the prompt before loading the prelude would be fine for now, it still better than wait before the prompt^^.

EDIT: This works fine with parallelism enabled! (flag -Dpreview_mt) Prelude is loaded in background while you can still edit the expression :grinning:.

The only thing to do then is to ensure than prelude is loaded before interpreting the code.

Vici37 commented 2 years ago

Ah, good explanation with the REPL behavior - I noticed it after I set up the PR, but still found it a better user experience than a delayed initial prompt displaying :)

Great find with the parallel loading! I'll experiment with thi, channels could probably be uses here to halt interpretation until loading is complete from the other thread. I'll try and update this PR tomorrow if I have time.

Vici37 commented 2 years ago

For testing, I found p to be the shortest valid command I could enter.

I3oris commented 2 years ago

Woaw great, thank you for the PR!

I3oris commented 2 years ago

Loving see the interpreter load instant' ! :tada:

Vici37 commented 2 years ago

Merge at your leisure, unless you have review comments I missed, I have nothing more to add :D

I3oris commented 2 years ago

Oh!, actually I have, I didn't realize that my review was hidden! I didn't know that pending review wasn't public, my bad.

Vici37 commented 2 years ago

Updated! Also cleaned up the commit history a bit

I3oris commented 2 years ago

Thank you Vici37! That's perfect!