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

Issue 12876047: code review 12876047: time: lower level interface to Timer: embedding, compac...

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years ago by funny.falcon
Modified:
5 years, 5 months ago
CC:
golang-codereviews
Visibility:
Public.

Description

time: lower level interface to Timer: embedding, compact interface callback with fast callback In developed library, there is a need to setup timer per every outgoing request. Allocating timer separately costs too much. Allocation func() for request's method also costs. So that, embedding Timer into request struct and setting callback by pointer to struct (which implements Expirator) give countable improvement. BenchmarkPointerStop 5000000 297 ns/op BenchmarkPointerStop-2 10000000 266 ns/op BenchmarkPointerStop-3 10000000 289 ns/op BenchmarkPointerStop-4 5000000 317 ns/op BenchmarkEmbedStop 10000000 176 ns/op BenchmarkEmbedStop-2 10000000 223 ns/op BenchmarkEmbedStop-3 10000000 266 ns/op BenchmarkEmbedStop-4 10000000 293 ns/op

Patch Set 1 #

Patch Set 2 : diff -r 9810b5abd0ab https://code.google.com/p/go #

Patch Set 3 : diff -r 9810b5abd0ab https://code.google.com/p/go #

Patch Set 4 : diff -r 9810b5abd0ab https://code.google.com/p/go #

Patch Set 5 : diff -r 9810b5abd0ab https://code.google.com/p/go #

Total comments: 1

Patch Set 6 : diff -r 9810b5abd0ab https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+89 lines, -0 lines) Patch
M src/pkg/runtime/time.goc View 1 1 chunk +1 line, -0 lines 0 comments Download
M src/pkg/time/sleep.go View 1 2 3 4 5 1 chunk +49 lines, -0 lines 0 comments Download
M src/pkg/time/sleep_test.go View 1 2 3 4 5 1 chunk +39 lines, -0 lines 0 comments Download

Messages

Total messages: 28
funny.falcon
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
6 years ago (2013-08-23 17:50:29 UTC) #1
funny.falcon
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), Please take another look.
5 years, 12 months ago (2013-08-23 20:18:10 UTC) #2
bradfitz
R=dvyukov This CL isn't acceptable as-is (too much new weird API, and docs need lot ...
5 years, 12 months ago (2013-08-23 20:28:21 UTC) #3
remyoudompheng
https://codereview.appspot.com/12876047/diff/10001/src/pkg/time/sleep.go File src/pkg/time/sleep.go (right): https://codereview.appspot.com/12876047/diff/10001/src/pkg/time/sleep.go#newcode137 src/pkg/time/sleep.go:137: // SetupExpire initialize already allocated timer to call e.Expire() ...
5 years, 12 months ago (2013-08-23 20:28:59 UTC) #4
funny.falcon
On 2013/08/23 20:28:59, remyoudompheng wrote: > https://codereview.appspot.com/12876047/diff/10001/src/pkg/time/sleep.go > File src/pkg/time/sleep.go (right): > > https://codereview.appspot.com/12876047/diff/10001/src/pkg/time/sleep.go#newcode137 > ...
5 years, 12 months ago (2013-08-23 21:33:30 UTC) #5
bradfitz
Yuri, We do want things to be very fast, but we also want clean, simple ...
5 years, 12 months ago (2013-08-23 21:40:46 UTC) #6
funny.falcon
Brad, Dmitry I've separate EmbedTimer from Timer, so that new methods do not pollute Timer's ...
5 years, 12 months ago (2013-08-24 06:54:55 UTC) #7
remyoudompheng
On 2013/8/24 <funny.falcon@gmail.com> wrote: > Brad, Dmitry > > I've separate EmbedTimer from Timer, so ...
5 years, 12 months ago (2013-08-24 07:10:44 UTC) #8
dvyukov
Hi, I agree with others that such a low-level interface is not a good thing. ...
5 years, 12 months ago (2013-08-24 10:45:57 UTC) #9
dvyukov
On 2013/08/23 21:33:30, funny.falcon wrote: > On 2013/08/23 20:28:59, remyoudompheng wrote: > > https://codereview.appspot.com/12876047/diff/10001/src/pkg/time/sleep.go > ...
5 years, 12 months ago (2013-08-24 10:48:02 UTC) #10
funny.falcon
On 2013/08/24 07:10:44, remyoudompheng wrote: > On 2013/8/24 <mailto:funny.falcon@gmail.com> wrote: > > Brad, Dmitry > ...
5 years, 12 months ago (2013-08-24 11:18:27 UTC) #11
funny.falcon
On 2013/08/24 10:45:57, dvyukov wrote: > Hi, > > I agree with others that such ...
5 years, 12 months ago (2013-08-24 11:22:25 UTC) #12
dvyukov
On 2013/08/24 11:22:25, funny.falcon wrote: > On 2013/08/24 10:45:57, dvyukov wrote: > > Hi, > ...
5 years, 12 months ago (2013-08-24 12:04:07 UTC) #13
funny.falcon
On 2013/08/24 12:04:07, dvyukov wrote: > On 2013/08/24 11:22:25, funny.falcon wrote: > > On 2013/08/24 ...
5 years, 12 months ago (2013-08-24 15:33:52 UTC) #14
dvyukov
On Sat, Aug 24, 2013 at 7:33 PM, <funny.falcon@gmail.com> wrote: > On 2013/08/24 12:04:07, dvyukov ...
5 years, 12 months ago (2013-08-24 15:55:17 UTC) #15
funny.falcon
On 2013/08/24 15:55:17, dvyukov wrote: > On Sat, Aug 24, 2013 at 7:33 PM, <mailto:funny.falcon@gmail.com> ...
5 years, 12 months ago (2013-08-26 08:53:36 UTC) #16
rsc
NOT LGTM Too much new API. Timers can already be reused.
5 years, 11 months ago (2013-09-03 21:34:38 UTC) #17
funny.falcon
On 2013/09/03 21:34:38, rsc wrote: > NOT LGTM > > Too much new API. Timers ...
5 years, 11 months ago (2013-09-05 15:38:09 UTC) #18
rsc
http://play.golang.org/p/Jw3YEDMY6d
5 years, 11 months ago (2013-09-05 16:27:59 UTC) #19
funny.falcon
Russ, Now 100_000 of them per second? How much presure on GC it will cause? ...
5 years, 11 months ago (2013-09-05 21:46:18 UTC) #20
rsc
On Thu, Sep 5, 2013 at 5:46 PM, Юрий Соколов <funny.falcon@gmail.com> wrote: > Russ, > ...
5 years, 11 months ago (2013-09-05 23:08:23 UTC) #21
funny.falcon
Then, I need to create 100_000 of closures per sec? Ah, callback could call interface, ...
5 years, 11 months ago (2013-09-06 06:47:29 UTC) #22
remyoudompheng
On 2013/9/6 Юрий Соколов <funny.falcon@gmail.com> wrote: > Then, I need to create 100_000 of closures ...
5 years, 11 months ago (2013-09-06 07:01:14 UTC) #23
dvyukov
That's pretty much what I showed here: http://play.golang.org/p/HVilcEF0qt You have a pair of (timer, chan), ...
5 years, 11 months ago (2013-09-06 08:14:46 UTC) #24
dvyukov
Note that Russ' code needs additional synchronization, as-is it contains a data race on f. ...
5 years, 11 months ago (2013-09-06 08:15:18 UTC) #25
funny.falcon
Dmitry, your code needs synchronization as well, cause this is not safe in case GOMAXPROCS ...
5 years, 11 months ago (2013-09-06 09:12:56 UTC) #26
gobot
Replacing golang-dev with golang-codereviews.
5 years, 8 months ago (2013-12-20 16:25:57 UTC) #27
rsc
5 years, 5 months ago (2014-03-05 20:27:02 UTC) #28
R=close
Sign in to reply to this message.

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