gmane.comp.lang.go.devel

Re: code review 9459046: codereview: add khr as a person (issue 9459046)

2013-05-21T15:55:29

LGTM

https://codereview.appspot.com/9459046/

Re: code review 8651043: lib9/utf: remove unneccesary includes (issue 8651043)

2013-05-21T15:50:59

pong


https://codereview.appspot.com/8651043/

Re: code review 9605043: misc/emacs: Do not modify kill ring when programmatical... (issue 9605043)

2013-05-21T15:50:00

R=adonovan (assigned by bradfitz)

https://codereview.appspot.com/9605043/

Re: code review 9598045: go/test/bench/go1: enable benchmarking on 32-bit machines (issue 9598045)

2013-05-21T15:49:27

R=cshapiro (assigned by bradfitz)

https://codereview.appspot.com/9598045/

code review 9459046: codereview: add khr as a person (issue 9459046)

2013-05-21T15:42:07

Reviewers: golang-dev1,

Message:
Hello golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
codereview: add khr as a person

Please review this at https://codereview.appspot.com/9459046/

Affected files:
   M misc/dashboard/codereview/dashboard/people.go


Index: misc/dashboard/codereview/dashboard/people.go
===================================================================
--- a/misc/dashboard/codereview/dashboard/people.go
+++ b/misc/dashboard/codereview/dashboard/people.go
< at >< at > -28,6 +28,7 < at >< at >
  "dsymonds",
  "gri",
  "iant",
+"khr",
  "mpvl",
  "nigeltao",
  "r",

Re: Proposal, Len interface

Ryan Slade — 2013-05-21T15:41:27

Nice, I didn't realise you could declare anonymous interfaces like that, 
thanks for the tip.

On Tuesday, 21 May 2013 16:34:07 UTC+1, Brad Fitzpatrick wrote:

Re: code review 9543043: database/sql: remove extra RemoveDep call (issue 9543043)

2013-05-21T15:37:27

R=adg (assigned by bradfitz)

https://codereview.appspot.com/9543043/

Re: code review 9570043: net/http: add another Hijacker test (issue 9570043)

2013-05-21T15:37:41

R=dsymonds (assigned by bradfitz)

https://codereview.appspot.com/9570043/

Re: linux-arm-armv5t broken by runtime: revert 3c2cddfbdaec

Shane Hansen — 2013-05-21T15:35:43

Sorry, I have this automatically filtered and I've got about 2,047 messages
in my golang-dev folder.
I'm on it team. Just has to wait until I get home because I changed routers
4 days ago and I don't have any
sort of port forwarding/remote access set up.

Should be just a matter of rebooting the node. I might as well rebuild the
builder while I'm at it since it's a couple months old.

Cheers,
--Shane Hansen


On Fri, May 17, 2013 at 2:38 AM, Dave Cheney <dave-7L4Cwp9BzA+sTnJN9+BGXg< at >public.gmane.org> wrote:

Re: Proposal, Len interface

Brad Fitzpatrick — 2013-05-21T15:34:07

I could have also just written:

if ln, ok := body.(interface { Len() int }); ok {
    req.ContentLength = int64(ln.Len())
}

Without introducing Lenner.

But the code conservatively only wanted to allow those named types.




On Tue, May 21, 2013 at 8:21 AM, Ryan Slade <ryanslade-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org> wrote:

Re: code review 9612044: sort: provide different stable sort algorithms (issue 9612044)

Brad Fitzpatrick — 2013-05-21T15:31:17

Excellent, thanks for working on this!

I'm worried about GPL infection.  Could you compare stl_algo.h from an SGI
or HP STL, before it went into GNU's libstdc++ to make sure it's the same,
back before it got a GPL sticker on it too?



On Tue, May 21, 2013 at 2:09 AM, <dr.volker.dobler-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org> wrote:

Re: code review 9608043: net/http: use WriteString directly when possible (issue 9608043)

Brad Fitzpatrick — 2013-05-21T15:25:31


Either way. But neither would be near the top of my static analysis
optimization/warning wishlist.

This wasn't a performance problem. It was like 0.3% CPU spent inside
io.WriteString, but I noticed it as unnecessary clutter in the profile.

Re: whither godoc

Brad Fitzpatrick — 2013-05-21T15:23:42



Unknown. I don't see any short-term moves, though. The main priority was
finding a home for types, ssa, and vet.

I imagine the go tool will always stay in main (it's used for bootstrap,
after all), but it might have subcommands implemented in other subrepos.

Re: code review 9459044: bufio: reuse Writer buffers after Flush (issue 9459044)

2013-05-21T15:22:46

"A bufio.Writer.Flush marks the usual end of a Writer's life. "

This is also often not the case. I have some programs that need to keep 
transaction logs. These programs have *bufio.Writers with large buffers, 
64, 128 MB. To maintain consistency these programs will often flush their 
*bufio.Writer and sync the underlying file at set intervals, 5 seconds, 30 
seconds, etc. I worry about this proposed change causing increased GC 
activity in these kind of programs (assuming this large buffers are not 
added to the buffer pool). I also have network communications that are 
buffered that often use Flush, only to immediately start writing data again.

Would it be possible to note, with a bool field or something, if the Writer 
had to reallocate a buffer after its last Flush(), and if so disable the 
new behaviour? This would allow the new behaviour without adding knobs or 
forcing people to write their own buffers. Another idea would be to not 
recycle buffers that are larger than a given size, say 1 MB, perhaps this 
is already the case.

On Monday, May 20, 2013 7:01:19 PM UTC-5, Brad Fitzpatrick wrote:

Proposal, Len interface

Ryan Slade — 2013-05-21T15:21:55

Hi

I was looking at this code:
(The if body != nil type switch)
https://code.google.com/p/go/source/browse/src/pkg/net/http/request.go#446

This code could be simplified if we had a "Lenner" interface:
type Lenner() interface {
    Len() int
}

We could then simply have something like:
if body != nil {
if l, ok := body.(Lenner); ok {
req.ContentLength = int64(l.Len())
}
}

However, where would be the best place to define this interface? Is it 
worth doing?

As a side effect, doing it this way appeared to give a slight performance 
improvement.

I haven't looked further into the issue as there may have been a good 
reason this wasn't done before, but I'm sure there may be other places in 
the code where having a Lenner interface could at least remove some 
repetitive code.

I'd love to hear if anyone thinks this is worthwhile doing before I submit 
a code review.

Thanks
Ryan

Re: code review 9462044: io: Prioritize WriterTos over ReaderFroms in Copy. (issue 9462044)

2013-05-21T15:20:47

LGTM

I've been trying to think of compatibility problems this could
introduce, but I haven't been able to.

We can always revert this if we think of something.


https://codereview.appspot.com/9462044/

Re: code review 9519044: api/next.txt: update (issue 9519044)

Brad Fitzpatrick — 2013-05-21T15:07:11

LGTM
 On May 21, 2013 6:56 AM, <minux.ma-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org> wrote:

Re: Fwd: sync.Pool

Sameer Ajmani — 2013-05-21T14:52:58


If this pool is just for managing allocations (as opposed to DB
connections, say), then package alloc, type Pool.

Re: code review 9539045: crypto/tls: don't send NPN extension if NextProtos is n... (issue 9539045)

2013-05-21T14:47:39

*** Submitted as
https://code.google.com/p/go/source/detail?r=08896028782c ***

crypto/tls: don't send NPN extension if NextProtos is not set.

This isn't clearly a bug on Go's part, but it triggers a bug in Firefox
which means that crypto/tls and net/http cannot be wired up together
unless NextProtos includes "http/1.1". When net/http sets up the
tls.Config, it does this and so works fine. But anyone setting up the
tls.Config themselves will hit the Firefox bug.

Fixes issue 5445.

R=golang-dev, bradfitz, r
CC=golang-dev
https://codereview.appspot.com/9539045


https://codereview.appspot.com/9539045/

Re: Fwd: sync.Pool

Dmitry Vyukov — 2013-05-21T14:07:42

Any suggestions on naming?

Re: Fwd: sync.Pool

Ian Lance Taylor — 2013-05-21T14:04:56

It seems to me that we would want to use entirely different
implementations for those.


That's a good reason to pick a good package name.  E.g., I'm not sure
why this Pool should be in the sync package.

Ian