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

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, 1 month ago by funny.falcon
Modified:
5 years, 7 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, 1 month 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.
6 years, 1 month 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 ...
6 years, 1 month 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() ...
6 years, 1 month 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 > ...
6 years, 1 month 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 ...
6 years, 1 month 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 ...
6 years, 1 month 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 ...
6 years, 1 month 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. ...
6 years, 1 month 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 > ...
6 years, 1 month 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 > ...
6 years, 1 month 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 ...
6 years, 1 month 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, > ...
6 years, 1 month 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 ...
6 years, 1 month 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 ...
6 years, 1 month 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> ...
6 years, 1 month ago (2013-08-26 08:53:36 UTC) #16
rsc
NOT LGTM Too much new API. Timers can already be reused.
6 years, 1 month 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 ...
6 years, 1 month ago (2013-09-05 15:38:09 UTC) #18
rsc
http://play.golang.org/p/Jw3YEDMY6d
6 years, 1 month 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? ...
6 years, 1 month 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, > ...
6 years, 1 month 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, ...
6 years, 1 month 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 ...
6 years, 1 month 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), ...
6 years, 1 month 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. ...
6 years, 1 month 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 ...
6 years, 1 month ago (2013-09-06 09:12:56 UTC) #26
gobot
Replacing golang-dev with golang-codereviews.
5 years, 9 months ago (2013-12-20 16:25:57 UTC) #27
rsc
5 years, 7 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