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

Busy wait in Task.init I/O causes cpu spike #1348

Merged
merged 2 commits into from Dec 8, 2015

Conversation

Projects
None yet
2 participants
@etcimon
Contributor

etcimon commented Dec 8, 2015

Avoid a busy wait when running I/O outside fibers.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Dec 8, 2015

Member

Does libasync recognize Duration.max there`? That would be the correct semantics in this case. 100 ms is functionally fine, too, but it would waste energy/battery if the process is idle.

Member

s-ludwig commented Dec 8, 2015

Does libasync recognize Duration.max there`? That would be the correct semantics in this case. 100 ms is functionally fine, too, but it would waste energy/battery if the process is idle.

@etcimon

This comment has been minimized.

Show comment
Hide comment
@etcimon

etcimon Dec 8, 2015

Contributor

Yes, I'm only worried if the task does a yield(). That could potentially cause it to freeze. Do you think this should be specifically forbidden just in case?

Contributor

etcimon commented Dec 8, 2015

Yes, I'm only worried if the task does a yield(). That could potentially cause it to freeze. Do you think this should be specifically forbidden just in case?

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Dec 8, 2015

Member

I don't understand, in which case would it freeze? Everytime at least one task calls yield() it must be guaranteed that onIdle will be called before going to sleep again (i.e. whenever the event queue gets empty). This should avoid any freezes.

Member

s-ludwig commented Dec 8, 2015

I don't understand, in which case would it freeze? Everytime at least one task calls yield() it must be guaranteed that onIdle will be called before going to sleep again (i.e. whenever the event queue gets empty). This should avoid any freezes.

@etcimon

This comment has been minimized.

Show comment
Hide comment
@etcimon

etcimon Dec 8, 2015

Contributor

I think it froze if yield was called twice in a row, because it won't loop back into notifyIdle, which is why I removed those in favor of manual events in the http/2 driver.

Contributor

etcimon commented Dec 8, 2015

I think it froze if yield was called twice in a row, because it won't loop back into notifyIdle, which is why I removed those in favor of manual events in the http/2 driver.

@etcimon

This comment has been minimized.

Show comment
Hide comment
@etcimon

etcimon Dec 8, 2015

Contributor

So I think this must have been a false conclusion I made back when notifyIdle wasn't called in the right places in libasync. I think Duration.max is safe to use in that case

Contributor

etcimon commented Dec 8, 2015

So I think this must have been a false conclusion I made back when notifyIdle wasn't called in the right places in libasync. I think Duration.max is safe to use in that case

@etcimon

This comment has been minimized.

Show comment
Hide comment
@etcimon

etcimon Dec 8, 2015

Owner

in the posix driver for libasync, the current behavior is to cast the msecs total into int. I'm not sure if this would cause problems for long.max

Owner

etcimon commented on b25e844 Dec 8, 2015

in the posix driver for libasync, the current behavior is to cast the msecs total into int. I'm not sure if this would cause problems for long.max

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Dec 8, 2015

Member

OK

Member

s-ludwig commented Dec 8, 2015

OK

s-ludwig added a commit that referenced this pull request Dec 8, 2015

Merge pull request #1348 from etcimon/patch-29
Busy wait in Task.init I/O causes cpu spike

@s-ludwig s-ludwig merged commit 97804cc into vibe-d:master Dec 8, 2015

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Dec 8, 2015

Member

Now LibasyncDriver.exitEventLoop just needs something to nudge the event loop.

Member

s-ludwig commented Dec 8, 2015

Now LibasyncDriver.exitEventLoop just needs something to nudge the event loop.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Dec 8, 2015

Member

Seems to be more than that - dub test :core -c libasync hangs after "Acquire event ID#32".

Member

s-ludwig commented Dec 8, 2015

Seems to be more than that - dub test :core -c libasync hangs after "Acquire event ID#32".

@etcimon

This comment has been minimized.

Show comment
Hide comment
@etcimon

etcimon Dec 8, 2015

Contributor

Maybe just AsyncNotifier with no handler. Strange that the event would make it hang...

Contributor

etcimon commented Dec 8, 2015

Maybe just AsyncNotifier with no handler. Strange that the event would make it hang...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment