WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

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.

3 participants