Skip to content
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

dotnet-httprepl prints usage very slowly, repeats characters in Linux #15

Closed
jdmallen opened this issue Sep 17, 2018 · 14 comments
Closed
Labels
backlog backlog

Comments

@jdmallen
Copy link

Environment

dotnet --info Output:

.NET Core SDK (reflecting any global.json):
 Version:   2.2.100-preview2-009404
 Commit:    f4707d384d

Runtime Environment:
 OS Name:     ubuntu
 OS Version:  18.04
 OS Platform: Linux
 RID:         ubuntu.18.04-x64
 Base Path:   /home/jesse/dotnet/sdk/2.2.100-preview2-009404/

Host (useful for support):
  Version: 2.2.0-preview2-26905-02
  Commit:  ad4d306fe0

.NET Core SDKs installed:
  2.2.100-preview2-009404 [/home/jesse/dotnet/sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.2.0-preview2-35157 [/home/jesse/dotnet/shared/Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.2.0-preview2-35157 [/home/jesse/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.2.0-preview2-26905-02 [/home/jesse/dotnet/shared/Microsoft.NETCore.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download

Note: dotnet points to preview version on my machine.

Slow terminal printing

  1. I installed latest dotnet-httprepl with this command:
    dotnet tool install -g dotnet-httprepl --version 2.2.0-preview3-35246 --add-source https://dotnet.myget.org/F/dotnet-core/api/v3/index.json

  2. I created and ran a default webapi project: dotnet new webapi && dotnet run

  3. I spun up dotnet-httprepl with dotnet-httprepl --help

The usage printed to the screen one line at a time extremely slowly. I timed it and it takes a full 10 seconds to print to the terminal.

Duplicated characters

The second issue is when I open a repl session, typing results in duplicate characters to be printed to the screen:

  1. Run httprepl with dotnet-httprepl http://localhost:5000
jesse@xubuntu:~/git $ dotnet-httprepl http://localhost:5000
(Disconnected)~ set base "http://localhost:5000"

http://localhost:5000/~ lls

http://localhost:5000/~ II  am ttypiing a senttencce

No matching command found
Execute 'help' to se available commands

http://localhost:5000/~ II  aam  ccarefully ttypingg  tthis..  Myy  kkeyboard is not brokenn.e

No matching command found
Execute 'help' to se available commands

http://localhost:5000/~ sset  bbaase  hhttttppss::////lloocalhost::550001

https://localhost:5001/~

As you can see, it still understands the commands that are being typed, so I think it's just a display issue.

Also that string "Execute 'help' to se available commands" was copied verbatim from the output.

I'm looking forward to using this tool in my development! :)

@bradygaster
Copy link
Member

@glennc just a note here - @mlorbetske confirmed this issue isn't resident in anything other than 18.04 and that tests validated this. That said, 18.04 is going to become the LTS for Ubuntu so this is still a thing.

@tomchavakis
Copy link

The same behavior occurred at Kubuntu 18.04 at 2.2.0-preview3-35497 version.

@mlorbetske
Copy link

I started looking at this yesterday, it seems we need analogous tweaks to the stty settings for the REPL session like we do for mac. The slow printing seems to be coming from repeatedly accessing CursorTop and CursorLeft in MoveCaret causing it to run in to this issue. I'll be testing some changes that reduce the number of accesses significantly soon.

mlorbetske referenced this issue in mlorbetske/DotNetTools Oct 30, 2018
Fix aspnet#503

Fix aspnet#492

Fix aspnet#491

Fix aspnet#486

Changes to improve #489
mlorbetske referenced this issue in mlorbetske/DotNetTools Oct 31, 2018
Fix aspnet#503

Fix aspnet#492

Fix aspnet#491

Fix aspnet#486

Changes to improve #489
mlorbetske referenced this issue in mlorbetske/DotNetTools Oct 31, 2018
mlorbetske referenced this issue in mlorbetske/DotNetTools Oct 31, 2018
mlorbetske referenced this issue in aspnet/DotNetTools Nov 1, 2018
Fix #502

Fix #492

Fix #491

Fix #486

Improves #489
@mlorbetske
Copy link

The duplicated characters has been fixed in #507, however the slow printing is still happening

@bradygaster
Copy link
Member

Thanks for the update @mlorbetske - keep us posted and if we need to sync up or you uncover anything about which you're concerned during the fix ping us. cc @glennc

@vijayrkn
Copy link

vijayrkn commented Nov 7, 2018

@natemcmaster natemcmaster transferred this issue from aspnet/DotNetTools Nov 20, 2018
@stephentoub
Copy link
Member

@bradygaster, @mlorbetske, @karelz, @wtgodbe, since our meeting ended early, I used the refund to take a quick look at the "Slow terminal printing" issue.

For me it's taking 6 seconds to output the result of "help", due to it making ~260 calls to get the current cursor position (which is the vast majority of the time), just to output the ~40 lines of help output (which, coincidentally or not, has ~260 words in it).

I think the fix here needs to be stop getting the current cursor position so frequently: it should not be needed at all for output like this.

We could of course look at optimizing the method that gets the current position, but fundamentally that method needs to write to the terminal, wait for the terminal to respond, read the terminal's response from stdin, and parse that response in order to pull out the relevant position information, and that's never going to be super fast; any optimization we did we just be removing some small overheads in the reading/writing/parsing. The only thing I can see that would make it orders of magnitude faster were if we were to cache the information and only get it again when it was invalidated by some external signal, which as we've talked about is going to be difficult to do reliably.

@tlmii tlmii transferred this issue from aspnet/AspLabs Jun 18, 2019
@tlmii
Copy link
Member

tlmii commented Jul 9, 2019

As far as I can tell, between the various commits mlorbetske made back in October, the changes stephentoub made to help in February and the changes being made in https://github.com/dotnet/corefx/issues/31517, the only thing we're really waiting on here is the release of the changes in that latter issue with 3.0. But the two overall issues are essentially fixed as far as our code goes.

@jodavis do you think this is safe to close?

@jodavis
Copy link
Contributor

jodavis commented Jul 10, 2019

What's the experience now on Linux? Especially if we run the tool on .NET Core 2.2 (which will probably never have the fix for https://github.com/dotnet/corefx/issues/31517)?

@jdmallen
Copy link
Author

jdmallen commented Jul 10, 2019

I can try this later this evening and report back. I have the same environment available in which I originally discovered this issue. Thanks for everyone's hard work on this. :)

@tlmii
Copy link
Member

tlmii commented Jul 10, 2019

Actually, doing a quick test on Ubuntu 18.04 (in WSL2), it doesn't look like the duplicate characters thing has actually been fixed. The speed of printing is fine though (help prints almost instantaneously.

Update: This appears to be an issue with OS detection. We're checking RuntimeInformation.OSDescription for buntu, but on Ubuntu 18.04 on WSL2 (not sure about WSL1), this returns

Linux 4.19.43-microsoft-standard # 1 SMP Mon May 20 19:35:22 UTC 2019

So it looks like we need another way of detecting Ubuntu (and whatever other affecting distros there might be).

@tlmii
Copy link
Member

tlmii commented Jul 10, 2019

Dan pointed us in the right direction over here: https://github.com/dotnet/corefx/issues/39366#issuecomment-510244486 So I'll get that fix in tomorrow probably.

@tlmii
Copy link
Member

tlmii commented Jul 11, 2019

On further inspection on this, it appears that we may not need to do distro detection... so far, every distro I have tried (SLES 12, Ubuntu 16.04, Ubuntu 18.04 and RHEL 7.6) has the duplicate issue.

Long-term we may need to do something to allow users to override the stty settings in preferences. But so far, it seems safe to just apply the erase 0x08 werase 0x08 -echo -icanon whenever we see we're running on linux.

@bradygaster
Copy link
Member

This was fixed a bit back, closing now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog backlog
Projects
None yet
Development

No branches or pull requests

9 participants