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

Ugly Dart HTTP Server response #2773

Closed
DartBot opened this issue Apr 27, 2012 · 9 comments
Closed

Ugly Dart HTTP Server response #2773

DartBot opened this issue Apr 27, 2012 · 9 comments
Assignees
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io

Comments

@DartBot
Copy link

DartBot commented Apr 27, 2012

This issue was originally filed by TerryMits...@gmail.com


What steps will reproduce the problem?

According to my test of the time_server.dart sample server program (included in Dart Editor) using Apache MINA proxi (http://mina.apache.org/report/trunk/xref/org/apache/mina/example/proxy/), following problems were found:

  1. Compare Dart HTTP server response header (lines 012 through 027) with Chrome’s request header (lines 001 through 011). So many TCP packets are used for the header portion.
  2. Within the body portion, single chunk is transferred using five TCP packets (lines 028 through 048).
  3. time_server.dart transmits identical HTTP responses against both ‘GET’ and ‘GET /favicon.ico’ requests from the browser. (This is a trivial bug.)

Here is a report from the proxy:
001 581395 [NioProcessor-5] INFO MINA_proxy.AbstractProxyIoHandler - GET / HTTP/1.1
002 Host: localhost:12345
003 Connection: keep-alive
004 Cache-Control: max-age=0
005 User-Agent: Mozilla/5.0 (Windows NT 6.0) AppleWebKit/535.19 (KHTML, like Gecko) Chrome/18.0.1025.162 Safari/535.19
006 Accept: text/html,application/xhtml+xml,application/xml;q=0.9,/;q=0.8
007 Accept-Encoding: gzip,deflate,sdch
008 Accept-Language: ja,en-US;q=0.8,en;q=0.6
009 Accept-Charset: Shift_JIS,utf-8;q=0.7,*;q=0.3
010
011
012 581403 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler - HTTP/1.1
013 581404 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler - 200 OK
014 581404 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler -
015 content-type
016 581405 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler - :
017 581405 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler - text/html; charset=UTF-8
018
019 581405 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler - transfer-encoding:
020 581406 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler - chunked
021
022 581406 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler - connection
023 581406 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler - : keep-alive
024 581406 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler -
025
026 581406 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler -
027
028 581407 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler - 15B
029 581407 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler -
030
031 581408 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler - <html>
032 <style>
033 body { background-color: teal; }
034 p { ba
035 581408 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler - ckground-color: white; border-radius: 8px; border:solid 1px #­555; text-align: center; padding: 0.5em;
036 font-family: "Luc
037 581408 [NioProcessor-8] INFO MINA_proxy.AbstractProxyIoHandler - ida Grande", Tahoma; font-size: 18px; color: #­555; }
038 </style>
039 <body>
040 <br/><br/>
041 <p>Current time: 2012-04-27 19:09:52.836</p>
042 </body>
043 </html>
044
045 0
046
047 …….

What is the expected output? What do you see instead?

Number of TCP packets should be minimized for the best transmission efficiency. This will help minimizing the response time.

What version of the product are you using? On what operating system?

Dart Editor Version 0.1.0.201204111510, Build 6442
Dart SDK version 6314, Dartium version
Windows Vista, 32bits

Please provide any additional information below.

@dgrove
Copy link
Contributor

dgrove commented Apr 27, 2012

Added Area-IO, Triaged labels.

@sgjesse
Copy link
Contributor

sgjesse commented May 2, 2012

The current implementation of the Dart HTTP library has not yet been tuned for performance.

@DartBot
Copy link
Author

DartBot commented May 30, 2012

This comment was originally written by TerryMit...@gmail.com


In the _HttpHeaders#_write method of /dart/runtime/bin/http_impl.dart?r=7943, it appears that every call of connection._write(COMMASP), connection._write(CRLF), connection._write(data) and connection._write(_COLONSP) is initiating TCP segment transmission.

As for HTTP response body, buffered data should be transmitted when size of the stored data reached to the specified content length or the buffer size or the buffer was closed/flushed. Tomcat uses 8192 bytes as a default buffer and chunk size.

TCP is a heavy protocol. Best transmission efficiency will be achieved by minimizing the number of transmitted TCP segments.

Following is an output of Tomcat version of the time server. Response header and body are transmitted in a single TCP segment.

25382 [NioProcessor-6] INFO MINA_proxy.AbstractProxyIoHandler - HTTP/1.1 200 OK
Server: Apache-Coyote/1.1
Content-Type: text/html;charset=Windows-31J
Content-Length: 328
Date: Sat, 28 Apr 2012 12:57:16 GMT

<html>
<style>
body { background-color: teal; }
p { background-color: white; border-radius: 8px; border:solid 1px #­555; text-align: center; padding: 0.5em;
font-family: 'Lucida Grande', Tahoma; font-size: 18px; color: #­555; }
</style>
<body><br/><br/>
<p>Current time: Fri Apr 27 21:57:16 JST 2012</p>
</body></html>

@madsager
Copy link
Contributor

madsager commented Jun 7, 2012

Added this to the Later milestone.

@DartBot
Copy link
Author

DartBot commented Jul 24, 2012

This comment was originally written by TerryMitsuok...@gmail.com


WebSocket response has the same problem.

@andersjohnsen
Copy link

In the current implementation, we do the following for HTTP and WebSockets:

Http: We send the HTTP header as one buffer, no matter how big it is. The content is accumulated to chunks of up to 16k. If a single piece of content is sent, with a size larger than 4k, it'll be sent directly and not accumulated. This means that we'll buffer small pieces of content, but send large pieces of content by itself.

WebSocket: We send the WebSocket Frame header as one chunk, and the frame payload as another. That means we send only two pieces of data for each chunk sent.

We see some nice results for this, but we are not yet done tuning.


Set owner to @Skabet.
Added Started label.

@andersjohnsen
Copy link

We've reduced the number of packages and are now quite happy with the output. I'm closing this bug for now, but feel free to open it again, should you encounter a concrete example we don't handle very well.


Added Fixed label.

@DartBot
Copy link
Author

DartBot commented Jun 27, 2013

This comment was originally written by TerryMit...@gmail.com


It's great! Thank you.

@kevmoo
Copy link
Member

kevmoo commented May 14, 2014

Removed Area-IO label.
Added Area-Library, Library-IO labels.

@DartBot DartBot added Type-Defect area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io labels May 14, 2014
@DartBot DartBot added this to the Later milestone May 14, 2014
dart-bot pushed a commit that referenced this issue Dec 7, 2020
a7e448b2 pub upgrade --major-versions (#2784)
06bcff09 pub upgrade foo, only unlocks foo from pubspec.lock (#2781)
aa20a4bd Usage exceptions no stacktrace (#2776)
14e12bf6 Clarify error message when spawning a subprocess fails (#2763)
5ef0a0e7 Added a pub upgrade --nullsafety mode (#2741)
9cde2406 Fix pub remove --dry-run (#2774)
7426be94 Remove hack now issue has been fixed (#2773)
90a1f776 Limit retries in test to avoid timeout (#2765)
3aa327c7 Stop depending on .packages (#2764)

Change-Id: Ifd8a8402c4628e7278b6fedfd2ca7902004fa6e8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175241
Auto-Submit: Jonas Jensen <jonasfj@google.com>
Reviewed-by: Sigurd Meldgaard <sigurdm@google.com>
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
dart-bot pushed a commit that referenced this issue Dec 8, 2020
This reverts commit 41381e9.

Reason for revert: pub fails on startup if git is not installed; b/175080640

Original change's description:
> Roll pub
>
> a7e448b2 pub upgrade --major-versions (#2784)
> 06bcff09 pub upgrade foo, only unlocks foo from pubspec.lock (#2781)
> aa20a4bd Usage exceptions no stacktrace (#2776)
> 14e12bf6 Clarify error message when spawning a subprocess fails (#2763)
> 5ef0a0e7 Added a pub upgrade --nullsafety mode (#2741)
> 9cde2406 Fix pub remove --dry-run (#2774)
> 7426be94 Remove hack now issue has been fixed (#2773)
> 90a1f776 Limit retries in test to avoid timeout (#2765)
> 3aa327c7 Stop depending on .packages (#2764)
>
> Change-Id: Ifd8a8402c4628e7278b6fedfd2ca7902004fa6e8
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175241
> Auto-Submit: Jonas Jensen <jonasfj@google.com>
> Reviewed-by: Sigurd Meldgaard <sigurdm@google.com>
> Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>

TBR=sigurdm@google.com,jonasfj@google.com

Change-Id: I29b20b4a2dc037146dd27c8fe6e6ecce6833e7b6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175303
Reviewed-by: David Morgan <davidmorgan@google.com>
Commit-Queue: David Morgan <davidmorgan@google.com>
dart-bot pushed a commit that referenced this issue Dec 9, 2020
This is a reland of 41381e9

Contains one more fix:
8ea96121d fix tryGitCommand

Original change's description:
> Roll pub
>
> a7e448b2 pub upgrade --major-versions (#2784)
> 06bcff09 pub upgrade foo, only unlocks foo from pubspec.lock (#2781)
> aa20a4bd Usage exceptions no stacktrace (#2776)
> 14e12bf6 Clarify error message when spawning a subprocess fails (#2763)
> 5ef0a0e7 Added a pub upgrade --nullsafety mode (#2741)
> 9cde2406 Fix pub remove --dry-run (#2774)
> 7426be94 Remove hack now issue has been fixed (#2773)
> 90a1f776 Limit retries in test to avoid timeout (#2765)
> 3aa327c7 Stop depending on .packages (#2764)
>
> Change-Id: Ifd8a8402c4628e7278b6fedfd2ca7902004fa6e8
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175241
> Auto-Submit: Jonas Jensen <jonasfj@google.com>
> Reviewed-by: Sigurd Meldgaard <sigurdm@google.com>
> Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>

Change-Id: Id50e6eca0372860f7087882a5c67141def07031c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175309
Reviewed-by: Jonas Jensen <jonasfj@google.com>
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io
Projects
None yet
Development

No branches or pull requests

6 participants