113304 2013-03-26 06:26:58 -0700 SocketStreamHandle (Chromium port) should fully use IPC window in send() 2013-03-26 22:05:46 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 tyoshino tyoshino ricea tkent toyoshim webkit.review.bot yhirano oldest_to_newest 863616 0 tyoshino 2013-03-26 06:26:58 -0700 socket_stream of Chromium buffers send data up to 32KiB (exact) bytes. However, SocketStreamHandleInternal::send() method keeps in-flight send data not greater than m_maxPendingSendAllowed - 1 that is 32KiB - 1. This means that SocketStreamHandleInternal consumes the buffered data in SocketStreamHandleBase by 32KiB - 1. It makes memory copy operations unaligned unnecessarily. An experiment using the benchmark tool of pywebsocket and emulated 10ms RTT ($ sudo tc qdisc add dev lo root netem delay 10ms) showed this change improves send performance to ~3 times of current. Message| size | After Before KiB | (kB/s) (kB/s) -------+---------------------------------- 10 |50171.485 21000.82 50 |63327.149 22520.343 100 |61798.431 22664.896 500 |67949.569 23572.744 1000 |68817.204 23858.341 5000 |68817.204 23836.127 10000 |64160.401 23437.858 50000 |59362.319 22314.23 100000 |59918.081 23852.784 863617 1 195075 tyoshino 2013-03-26 06:31:21 -0700 Created attachment 195075 Patch 864056 2 195075 toyoshim 2013-03-26 15:22:57 -0700 Comment on attachment 195075 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195075&action=review I agree with this change. > Source/WebCore/platform/network/chromium/SocketStreamHandle.cpp:80 > if (m_pendingAmountSent + len >= m_maxPendingSendAllowed) Nits: Now, this condition can be '>' instead of '>='. 864058 3 toyoshim 2013-03-26 15:25:56 -0700 +tkent for sanctifying to land this change. 864199 4 195212 tyoshino 2013-03-26 20:50:29 -0700 Created attachment 195212 Patch 864200 5 tyoshino 2013-03-26 20:51:48 -0700 Thanks for review, Takashi. (In reply to comment #2) > Nits: Now, this condition can be '>' instead of '>='. done 864220 6 195212 tkent 2013-03-26 21:41:44 -0700 Comment on attachment 195212 Patch Looks ok 864224 7 195212 webkit.review.bot 2013-03-26 22:05:41 -0700 Comment on attachment 195212 Patch Clearing flags on attachment: 195212 Committed r146965: <http://trac.webkit.org/changeset/146965> 864225 8 webkit.review.bot 2013-03-26 22:05:46 -0700 All reviewed patches have been landed. Closing bug. 195075 2013-03-26 06:31:21 -0700 2013-03-26 20:50:26 -0700 Patch bug-113304-20130326222704.patch text/plain 2260 tyoshino U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ2ODU0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjI0MzM2NmQ3MzljN2E4 ZDBmZWQ4OGVkM2M3NWU4OWFhNmFjMGZiZS4uNzJkZTI3YjVhYjRmMjE5YTFiYzUyMTgxZWQxYThj ZDQ4YjRmYWM5YiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIzIEBACisyMDEzLTAzLTI2ICBUYWtl c2hpIFlvc2hpbm8gIDx0eW9zaGlub0BjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgTWFrZSBTb2Nr ZXRTdHJlYW1IYW5kbGUgKENocm9taXVtIHBvcnQpIGZ1bGx5IHVzZSBJUEMgd2luZG93IGluIHNl bmQoKQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTEz MzA0CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgc29j a2V0X3N0cmVhbSBvZiBDaHJvbWl1bSBidWZmZXJzIHNlbmQgZGF0YSB1cCB0byAzMktpQiAoZXhh Y3QpIGJ5dGVzLgorCisgICAgICAgIEhvd2V2ZXIsIFNvY2tldFN0cmVhbUhhbmRsZUludGVybmFs OjpzZW5kKCkgbWV0aG9kIG5vdyBrZWVwcyBpbi1mbGlnaHQKKyAgICAgICAgc2VuZCBkYXRhIG5v dCBncmVhdGVyIHRoYW4gbV9tYXhQZW5kaW5nU2VuZEFsbG93ZWQgLSAxIHRoYXQgaXMKKyAgICAg ICAgMzJLaUIgLSAxLiBUaGlzIG1lYW5zIHRoYXQgU29ja2V0U3RyZWFtSGFuZGxlSW50ZXJuYWwg Y29uc3VtZXMgdGhlCisgICAgICAgIGJ1ZmZlcmVkIGRhdGEgaW4gU29ja2V0U3RyZWFtSGFuZGxl QmFzZSBieSAzMktpQiAtIDEuIEl0IG1ha2VzIG1lbW9yeQorICAgICAgICBjb3B5IG9wZXJhdGlv bnMgdW5hbGlnbmVkIHVubmVjZXNzYXJpbHkuIEl0IHNob3VsZCBqdXN0IHVzZSB0aGUKKyAgICAg ICAgYWxsb3dlZCBzaXplIGZ1bGx5LgorCisgICAgICAgICogcGxhdGZvcm0vbmV0d29yay9jaHJv bWl1bS9Tb2NrZXRTdHJlYW1IYW5kbGUuY3BwOgorICAgICAgICAoV2ViQ29yZTo6U29ja2V0U3Ry ZWFtSGFuZGxlSW50ZXJuYWw6OnNlbmQpOgorICAgICAgICAoV2ViQ29yZSk6CisKIDIwMTMtMDMt MjIgIEhhamltZSBNb3JyaXRhICA8bW9ycml0YUBnb29nbGUuY29tPgogCiAgICAgICAgIExpc3Rl bmluZyB0b3VjaCBldmVudHMgb24gU2hhZG93Um9vdCBjYW4gY3Jhc2guCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL2Nocm9taXVtL1NvY2tldFN0cmVhbUhhbmRs ZS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL2Nocm9taXVtL1NvY2tldFN0 cmVhbUhhbmRsZS5jcHAKaW5kZXggYTk5NWRhMDYxMzVmMmZhNjI2ZjJkN2ExZmIxOGRjMDZlYmEz OGU2Ny4uMDRjNjUwNmNjMmU5N2EzMjQ0ZDI1OTI2Y2E1NDMzNzA0NzQ1ZDQ4YiAxMDA2NDQKLS0t IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0d29yay9jaHJvbWl1bS9Tb2NrZXRTdHJlYW1I YW5kbGUuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL25ldHdvcmsvY2hyb21pdW0v U29ja2V0U3RyZWFtSGFuZGxlLmNwcApAQCAtNzgsNyArNzgsNyBAQCBpbnQgU29ja2V0U3RyZWFt SGFuZGxlSW50ZXJuYWw6OnNlbmQoY29uc3QgY2hhciogZGF0YSwgaW50IGxlbikKICAgICAgICAg cmV0dXJuIDA7CiAgICAgfQogICAgIGlmIChtX3BlbmRpbmdBbW91bnRTZW50ICsgbGVuID49IG1f bWF4UGVuZGluZ1NlbmRBbGxvd2VkKQotICAgICAgICBsZW4gPSBtX21heFBlbmRpbmdTZW5kQWxs b3dlZCAtIG1fcGVuZGluZ0Ftb3VudFNlbnQgLSAxOworICAgICAgICBsZW4gPSBtX21heFBlbmRp bmdTZW5kQWxsb3dlZCAtIG1fcGVuZGluZ0Ftb3VudFNlbnQ7CiAKICAgICBpZiAobGVuIDw9IDAp CiAgICAgICAgIHJldHVybiBsZW47CkBAIC05OCw3ICs5OCw3IEBAIHZvaWQgU29ja2V0U3RyZWFt SGFuZGxlSW50ZXJuYWw6OmNsb3NlKCkKICAgICBpZiAobV9zb2NrZXQpCiAgICAgICAgIG1fc29j a2V0LT5jbG9zZSgpOwogfQotICAgIAorCiB2b2lkIFNvY2tldFN0cmVhbUhhbmRsZUludGVybmFs OjpkaWRPcGVuU3RyZWFtKFdlYktpdDo6V2ViU29ja2V0U3RyZWFtSGFuZGxlKiBzb2NrZXRIYW5k bGUsIGludCBtYXhQZW5kaW5nU2VuZEFsbG93ZWQpCiB7CiAgICAgTE9HKE5ldHdvcmssICJTb2Nr ZXRTdHJlYW1IYW5kbGVJbnRlcm5hbDo6ZGlkT3BlbiAlZCIsCg== 195212 2013-03-26 20:50:29 -0700 2013-03-26 22:05:41 -0700 Patch bug-113304-20130327124611.patch text/plain 2397 tyoshino U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ2ODU0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjI0MzM2NmQ3MzljN2E4 ZDBmZWQ4OGVkM2M3NWU4OWFhNmFjMGZiZS4uNzJkZTI3YjVhYjRmMjE5YTFiYzUyMTgxZWQxYThj ZDQ4YjRmYWM5YiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIzIEBACisyMDEzLTAzLTI2ICBUYWtl c2hpIFlvc2hpbm8gIDx0eW9zaGlub0BjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgTWFrZSBTb2Nr ZXRTdHJlYW1IYW5kbGUgKENocm9taXVtIHBvcnQpIGZ1bGx5IHVzZSBJUEMgd2luZG93IGluIHNl bmQoKQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTEz MzA0CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgc29j a2V0X3N0cmVhbSBvZiBDaHJvbWl1bSBidWZmZXJzIHNlbmQgZGF0YSB1cCB0byAzMktpQiAoZXhh Y3QpIGJ5dGVzLgorCisgICAgICAgIEhvd2V2ZXIsIFNvY2tldFN0cmVhbUhhbmRsZUludGVybmFs OjpzZW5kKCkgbWV0aG9kIG5vdyBrZWVwcyBpbi1mbGlnaHQKKyAgICAgICAgc2VuZCBkYXRhIG5v dCBncmVhdGVyIHRoYW4gbV9tYXhQZW5kaW5nU2VuZEFsbG93ZWQgLSAxIHRoYXQgaXMKKyAgICAg ICAgMzJLaUIgLSAxLiBUaGlzIG1lYW5zIHRoYXQgU29ja2V0U3RyZWFtSGFuZGxlSW50ZXJuYWwg Y29uc3VtZXMgdGhlCisgICAgICAgIGJ1ZmZlcmVkIGRhdGEgaW4gU29ja2V0U3RyZWFtSGFuZGxl QmFzZSBieSAzMktpQiAtIDEuIEl0IG1ha2VzIG1lbW9yeQorICAgICAgICBjb3B5IG9wZXJhdGlv bnMgdW5hbGlnbmVkIHVubmVjZXNzYXJpbHkuIEl0IHNob3VsZCBqdXN0IHVzZSB0aGUKKyAgICAg ICAgYWxsb3dlZCBzaXplIGZ1bGx5LgorCisgICAgICAgICogcGxhdGZvcm0vbmV0d29yay9jaHJv bWl1bS9Tb2NrZXRTdHJlYW1IYW5kbGUuY3BwOgorICAgICAgICAoV2ViQ29yZTo6U29ja2V0U3Ry ZWFtSGFuZGxlSW50ZXJuYWw6OnNlbmQpOgorICAgICAgICAoV2ViQ29yZSk6CisKIDIwMTMtMDMt MjIgIEhhamltZSBNb3JyaXRhICA8bW9ycml0YUBnb29nbGUuY29tPgogCiAgICAgICAgIExpc3Rl bmluZyB0b3VjaCBldmVudHMgb24gU2hhZG93Um9vdCBjYW4gY3Jhc2guCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL2Nocm9taXVtL1NvY2tldFN0cmVhbUhhbmRs ZS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL2Nocm9taXVtL1NvY2tldFN0 cmVhbUhhbmRsZS5jcHAKaW5kZXggYTk5NWRhMDYxMzVmMmZhNjI2ZjJkN2ExZmIxOGRjMDZlYmEz OGU2Ny4uZDAzNzMyNjdhZGNiMmMxMGM5OTg5NDQ2MWM0ZTY0YThkYmUwYWQ3NiAxMDA2NDQKLS0t IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0d29yay9jaHJvbWl1bS9Tb2NrZXRTdHJlYW1I YW5kbGUuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL25ldHdvcmsvY2hyb21pdW0v U29ja2V0U3RyZWFtSGFuZGxlLmNwcApAQCAtNzcsOCArNzcsOCBAQCBpbnQgU29ja2V0U3RyZWFt SGFuZGxlSW50ZXJuYWw6OnNlbmQoY29uc3QgY2hhciogZGF0YSwgaW50IGxlbikKICAgICAgICAg TE9HKE5ldHdvcmssICJtX3NvY2tldCBpcyBudWxsIHdoZW4gc2VuZGluZy4gSXQgc2hvdWxkIG5v dCBiZS4iKTsKICAgICAgICAgcmV0dXJuIDA7CiAgICAgfQotICAgIGlmIChtX3BlbmRpbmdBbW91 bnRTZW50ICsgbGVuID49IG1fbWF4UGVuZGluZ1NlbmRBbGxvd2VkKQotICAgICAgICBsZW4gPSBt X21heFBlbmRpbmdTZW5kQWxsb3dlZCAtIG1fcGVuZGluZ0Ftb3VudFNlbnQgLSAxOworICAgIGlm IChtX3BlbmRpbmdBbW91bnRTZW50ICsgbGVuID4gbV9tYXhQZW5kaW5nU2VuZEFsbG93ZWQpCisg ICAgICAgIGxlbiA9IG1fbWF4UGVuZGluZ1NlbmRBbGxvd2VkIC0gbV9wZW5kaW5nQW1vdW50U2Vu dDsKIAogICAgIGlmIChsZW4gPD0gMCkKICAgICAgICAgcmV0dXJuIGxlbjsKQEAgLTk4LDcgKzk4 LDcgQEAgdm9pZCBTb2NrZXRTdHJlYW1IYW5kbGVJbnRlcm5hbDo6Y2xvc2UoKQogICAgIGlmICht X3NvY2tldCkKICAgICAgICAgbV9zb2NrZXQtPmNsb3NlKCk7CiB9Ci0gICAgCisKIHZvaWQgU29j a2V0U3RyZWFtSGFuZGxlSW50ZXJuYWw6OmRpZE9wZW5TdHJlYW0oV2ViS2l0OjpXZWJTb2NrZXRT dHJlYW1IYW5kbGUqIHNvY2tldEhhbmRsZSwgaW50IG1heFBlbmRpbmdTZW5kQWxsb3dlZCkKIHsK ICAgICBMT0coTmV0d29yaywgIlNvY2tldFN0cmVhbUhhbmRsZUludGVybmFsOjpkaWRPcGVuICVk IiwK