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

exponential backoff implementation to avoid over-polling resources #847

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shirouto
Copy link
Contributor

Description

Very naive implementation of exponential backoff to mitigate the overpolling of resources in the RTS.

How Has This Been Tested?

Very rudimentary testing, mainly to check whether the approach does improve the Warp IO issues and overusing of the CPU cores. This is an experiment and definitely a WIP.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation change
  • Test suite change

Checklist:

  • I have read the CONTRIBUTING document.

@CLAassistant
Copy link

CLAassistant commented Aug 15, 2018

CLA assistant check
All committers have signed the CLA.

@rahulmutt
Copy link
Member

rahulmutt commented Aug 17, 2018

@shirouto Thank you so much for taking the time to implement this. I have spent a while thinking about this and the root problem is that we do too much polling and we shouldn't be. I went through all our uses of polling and a majority of them can be avoided and use instead Java's efficient built-in mechanisms like Object.wait() and Object.notify() for signaling. Now there are a couple of use cases that absolutely require polling (like for waitFuture) since they don't have support from the JVM to do otherwise and in that case I think the best way to go is to just allocate a dedicated thread that does the polling with some exponential backoff.

These changes will take time since there are a lot of moving parts and room for error, so we'll tackle these one-by-one after we have a more comprehensive test suite (the current one has very few tests that test the concurrent aspects of the runtime).

In the meantime, I think we can use your exponential backoff changes. Overall, it looks ok.

I just think we need to add a couple tests to make sure they don't break anything. You can add tests in tests/suite/* (just follow the convention). You can run the test suite like this:

stack build eta-test-suite && stack exec eta-test-suite -- -p [test-suite-name]

Say you create a new folder in tests/suite/backoff and want to run just the tests inside - you can replace [test-suite-name] with backoff. Let me know if you need anymore help with adding new tests.

This was referenced Aug 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants