-
-
Notifications
You must be signed in to change notification settings - Fork 318
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
Adding tcell as a dependency delays startup time #764
Comments
I suspect this is because of the (lowercase!) Notably, I haven't tried this and I'm just speculating here, but you could try forking tcell and changing the This article explains that all the On my laptop, the difference is much less dramatic, with your code snippet taking 26 milliseconds to run, and not importing tcell taking 2 milliseconds to run (median, measured 10 times with
|
I just tried removing the mutex locks, and it made no difference so disregard that... |
Uncontended locks are cheap, ... really cheap. I would not have expected that to be a problem. I think it's more likely this one:
TCELL_MINIMIZE will save the effort to build the look up table. That could probably have been deferred and done lazily instead of during initi(), as its pretty expensive to build it, but it makes a noticeable difference for heavy users of Unicode. |
I m mostly sure is some init function. On my side i tried the same program adding all the 4 deps of tcell and the script was fast. I still do not know how to use a profiler in go and i m not able to pinpoint the exact line |
my first gut feeling was the init of runewidth with creatlut but it isn't, i tried it, it adds like 4ms on my slow machine. I will try again. I ll make a test again thank you for the info |
Confirmed... its that look up table creation that is hurting. What I think I can do, as I said, is defer that to a start up. I could also defer it to a go routine as well. |
Its easy to test... simply time with "TCELL_MINIMIZE=1" vs not having it set. |
So its really a concern not just for emoji, but for cases where the user has characters that fall outside of a narrow range of ASCII. Thats where it can make a big difference. Ideally we would like that LUT to be pre-compiled, and just have our binaries be a MB bigger if needed. I might see if the owner of that repo is amenable. |
Actually, I'm not really seeing a performance benefit from calling the createLUT at all. Stay tuned, I think I'm just going to remove that call. |
We no longer bother to create the lookup table for runewidth. I had expected this to make a significant difference, but I'm finding that it does not, but it does make the startup time even for applications that only import and never create a screen, considerably worse. Dozens of milliseconds, even up to half a second. Applications that need this (persumably some heavy Emoji or EastAsian users) can solve for themselves by importing the runewidth package (same version) and calling the CreateLUT method.
Thank you that was fast. Thank you again |
Hello everyone, i m working on a command line utility with optional UI.
I migrated a setup that included the c library curses to tview that uses tcell.
After finishing the migration i noticed that the application was considerably slower at starting up:
So i digged into what it could be and i made this test program:
this software compiled to run on a linux32 env with a slow 800mhz arm core
/media/fat/Scripts# time zaparoo.sh hello slow world real 0m0.576s user 0m0.549s sys 0m0.022s
while if i remove the tcell import i get the following:
/media/fat/Scripts# time zaparoo.sh hello slow world real 0m0.010s user 0m0.006s sys 0m0.006s
To be clear the start up times when i m running the real code of the service is around 0.014 so very similar to an empty program run.
Just importing tcell without displaying anything creates a decent delay.
So i wanted to ask if this is necessary and if it can be patched maybe adding an optional lazy initialization that i can run whenever i need to start using tcell actually rather than just including in my binary.
Thank you
The text was updated successfully, but these errors were encountered: