hharnisc / hypercwd

Opens new tabs with the same directory as the current tab in Hyper
MIT License
384 stars 39 forks source link

Hyper Canary - Slow stream from running programs #37

Closed richrace closed 6 years ago

richrace commented 6 years ago

Setup: screen shot 2018-01-12 at 10 52 59

Issue: Visible slow down with streaming output from command or log files.

Test:

for ((i=1; i<=10000; i++)); do
   echo $i
done
hharnisc commented 6 years ago

Hey @richrace :wave:

Thank you for the report here, this one has been an issue for quite some time - this snippet is great for reproduce the issue (was using the yes command which would always cause it to hang :smile: ). Unfortunately with the current implementation of Hyper, we have to check the process (almost) every time the output changes on the console. So the trade off is efficiency vs usefulness. -- I need to double check what's different in the 2.X canary release, if that changed we might be able to fix this!

rauchg commented 6 years ago

Just ran into this as well. yes in Hyper canary is nearly native-like, but if I turn on hypercwd, it barely renders.

I have an idea for a new strategy: only obtain the cwd upon creating a new tab or window. Don't do anything when the terminal is rendering.

Esteban-Rocha commented 6 years ago

Same here got to deactivate until this is fixed

hharnisc commented 6 years ago

Grabbed some performance data from chrome dev tools. Comparing the performance of the following script (basically the same as what you shared @richrace) on Hyper 2.0.0-canary.14:

for i in `seq 1 10000`;
do
  echo $i
done

Note: these comparisons are done while collecting performance metrics so the values don't reflect the actual performance -- the difference in performance under the same conditions is what is important here.

Without hypercwd:

Render Time: ~359.8ms Min FPS: ~5

without-hyper-cwd

With hypercwd:

Render Time: ~60 Seconds (yes seconds!) Min FPS: 0

with-hypercwd

It's clear in this view how expensive the spawn is -- and much more of an issue when streaming logs etc.

spawn

It's much more clear what's happening under the hood -- and I've got a couple ideas on how to fix this. Thank you for the input everyone :raised_hands:

hharnisc commented 6 years ago

I implemented an edge trigger function -- think debounce, but runs the passed in function at beginning and the end. And got pretty much the same performance as hyper without the plug ins. I'll deploy this soon! cc/ @rauchg

Render Time: ~428.1ms Min FPS: ~4

image

This fix should dramatically improve performance on OS X + Windows

hharnisc commented 6 years ago

Just published 1.2.1 release if you'd like to give this a try @rauchg @richrace @Esteban-Rocha

richrace commented 6 years ago

Brilliant. That works a treat 👍. Thanks!

I've patch my local copy of https://github.com/henrikdahl/hyper-statusline with your changes and it fixes https://github.com/henrikdahl/hyper-statusline/issues/69 too!

bmrobin commented 6 years ago

I've patch my local copy of https://github.com/henrikdahl/hyper-statusline with your changes and it fixes henrikdahl/hyper-statusline#69 too!

@richrace would you mind submitting a PR with that change to the hyper-statusline repository? i don't see any activity there related to fixing this issue and if you already have @hharnisc 's solution implemented then we could get that merged into the other project as well.

Exomnius commented 6 years ago

@richrace is it possible that you share your fix for https://github.com/henrikdahl/hyper-statusline too?

j-f1 commented 5 years ago

@richrace If you open a PR on my fork, I’ll get it released to npm.

richrace commented 5 years ago

@j-f1 Hey, Sorry, but I don't use HyperJS anymore. And the "fix" I used didn't really solve it.

For example, the CWD in the status line would not update correctly and then be completely out of sync.