Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid the unnecessary wait in the first iteration of a loop #929

Merged
merged 1 commit into from Dec 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 30 additions & 9 deletions chromedp.go
Expand Up @@ -627,17 +627,38 @@ func (t Tasks) Do(ctx context.Context) error {
// been able to be written/tested.
func Sleep(d time.Duration) Action {
return ActionFunc(func(ctx context.Context) error {
// Don't use time.After, to avoid a temporary goroutine leak if
// ctx is cancelled before the timer fires.
t := time.NewTimer(d)
select {
case <-ctx.Done():
t.Stop()
return ctx.Err()
case <-t.C:
return sleepContext(ctx, d)
})
}

// sleepContext sleeps for the specified duration. It returns ctx.Err() immediately
// if the context is cancelled.
func sleepContext(ctx context.Context, d time.Duration) error {
timer := time.NewTimer(d)
select {
case <-ctx.Done():
if !timer.Stop() {
<-timer.C
}
return ctx.Err()
case <-timer.C:
return nil
})
}
}

// retryWithSleep reties the execution of the specified func until the func returns
// true (means to stop) or a non-nil error.
func retryWithSleep(ctx context.Context, d time.Duration, f func(ctx context.Context) (bool, error)) error {
for {
toStop, err := f(ctx)
if toStop || err != nil {
return err
}
err = sleepContext(ctx, d)
if err != nil {
return err
}
}
}

type cancelableListener struct {
Expand Down
14 changes: 8 additions & 6 deletions poll.go
Expand Up @@ -38,13 +38,15 @@ func (p *pollTask) Do(ctx context.Context) error {
execCtx runtime.ExecutionContextID
ok bool
)
for !ok {
select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(5 * time.Millisecond):
}

for {
_, _, execCtx, ok = t.ensureFrame()
if ok {
break
}
if err := sleepContext(ctx, 5*time.Millisecond); err != nil {
return err
}
}

if p.frame != nil {
Expand Down
20 changes: 7 additions & 13 deletions query.go
Expand Up @@ -155,16 +155,10 @@ func (s *Selector) Do(ctx context.Context) error {
if t == nil {
return ErrInvalidTarget
}
for {
select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(5 * time.Millisecond):
}

return retryWithSleep(ctx, 5*time.Millisecond, func(ctx context.Context) (bool, error) {
frame, root, execCtx, ok := t.ensureFrame()
if !ok {
continue
return false, nil
}

fromNode := s.fromNode
Expand Down Expand Up @@ -194,20 +188,20 @@ func (s *Selector) Do(ctx context.Context) error {

ids, err := s.by(ctx, fromNode)
if err != nil || len(ids) < s.exp {
continue
return false, nil
}
nodes, err := s.wait(ctx, frame, execCtx, ids...)
// if nodes==nil, we're not yet ready
if nodes == nil || err != nil {
continue
return false, nil
}
if s.after != nil {
if err := s.after(ctx, execCtx, nodes...); err != nil {
return err
return true, err
}
}
return nil
}
return true, nil
})
}

// selAsString forces sel into a string.
Expand Down