Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(182)

Issue 8179043: code review 8179043: net/textproto: reduce allocations in ReadMIMEHeader (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 7 months ago by bradfitz
Modified:
4 years, 4 months ago
Reviewers:
adg, rsc
CC:
golang-dev, r, iant, adg
Visibility:
Public.

Description

net/textproto: reduce allocations in ReadMIMEHeader ReadMIMEHeader is used by net/http, net/mail, and mime/multipart. Don't do so many small allocations. Calculate up front how much we'll probably need. benchmark old ns/op new ns/op delta BenchmarkReadMIMEHeader 8433 7467 -11.45% benchmark old allocs new allocs delta BenchmarkReadMIMEHeader 23 14 -39.13% benchmark old bytes new bytes delta BenchmarkReadMIMEHeader 1705 1343 -21.23%

Patch Set 1 #

Patch Set 2 : diff -r 192e257c6507 https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r 192e257c6507 https://go.googlecode.com/hg/ #

Total comments: 1

Patch Set 4 : diff -r 192e257c6507 https://go.googlecode.com/hg/ #

Patch Set 5 : diff -r 3a652b14dccc https://go.googlecode.com/hg/ #

Patch Set 6 : diff -r 64f7f4530ca8 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -2 lines) Patch
M src/pkg/net/textproto/reader.go View 1 2 3 4 3 chunks +45 lines, -2 lines 0 comments Download

Messages

Total messages: 16
bradfitz
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/
4 years, 7 months ago (2013-03-29 21:58:38 UTC) #1
r
this sets a precedent i'd rather leave unset. https://codereview.appspot.com/8179043/diff/4001/src/pkg/net/textproto/reader.go File src/pkg/net/textproto/reader.go (right): https://codereview.appspot.com/8179043/diff/4001/src/pkg/net/textproto/reader.go#newcode14 src/pkg/net/textproto/reader.go:14: "unsafe" ...
4 years, 7 months ago (2013-03-29 22:18:04 UTC) #2
bradfitz
App Engine trusts the standard library. Do you still want it in a separate file? ...
4 years, 7 months ago (2013-03-29 22:25:19 UTC) #3
bradfitz
Stand by. Moving it to a separate file. On Fri, Mar 29, 2013 at 3:25 ...
4 years, 7 months ago (2013-03-29 22:42:47 UTC) #4
bradfitz
Hello golang-dev@googlegroups.com, r@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
4 years, 7 months ago (2013-03-29 22:54:28 UTC) #5
bradfitz
PTAL Now it'll involve less paperwork. On Fri, Mar 29, 2013 at 3:54 PM, <bradfitz@golang.org> ...
4 years, 7 months ago (2013-03-29 22:55:35 UTC) #6
r
I believe encoding/gob is the only standard package that uses unsafe to speed up allocations, ...
4 years, 7 months ago (2013-03-29 23:07:24 UTC) #7
bradfitz
Maybe I can use gob instead. On Fri, Mar 29, 2013 at 4:07 PM, <r@golang.org> ...
4 years, 7 months ago (2013-03-29 23:12:57 UTC) #8
bradfitz
*** Abandoned ***
4 years, 7 months ago (2013-03-29 23:16:13 UTC) #9
iant
Let's not work around a language/runtime problem by importing unsafe (unless we're desperate).
4 years, 7 months ago (2013-03-29 23:21:27 UTC) #10
bradfitz
I moved my whining to Issue 1642. I was hoping this CL would be more ...
4 years, 7 months ago (2013-03-29 23:31:32 UTC) #11
bradfitz
Hello golang-dev@googlegroups.com, r@golang.org, iant@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
4 years, 4 months ago (2013-07-02 13:21:19 UTC) #12
bradfitz
PTAL, 3 months later. Now a small patch, using s[i:j:k] instead of unsafe. On Tue, ...
4 years, 4 months ago (2013-07-02 13:24:23 UTC) #13
adg
LGTM
4 years, 4 months ago (2013-07-03 04:15:12 UTC) #14
bradfitz
*** Submitted as https://code.google.com/p/go/source/detail?r=bb98e4c9b6b3 *** net/textproto: reduce allocations in ReadMIMEHeader ReadMIMEHeader is used by net/http, ...
4 years, 4 months ago (2013-07-03 05:37:30 UTC) #15
rsc
4 years, 4 months ago (2013-07-08 19:09:54 UTC) #16
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld c59f3c4