-
Notifications
You must be signed in to change notification settings - Fork 22
Fix bug in futures implementation and check for pe num in pool examples #279
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
base: main
Are you sure you want to change the base?
Conversation
Can you explain the bug and the fix here? Also update the name of the PR |
This is a bug with the system of tracking which futures are borrowed by other PEs. When a future is borrowed by another PE, the future is stored in a dictionary called borrowed_futures on the originating PE. The borrowing PE may be the same as the original PE, and both may be the same or different from the PE that originally made the future. Any future can be sent from one PE to another, creating a tree that describes which PEs are borrowing any future. The borrowed_futures dictionary makes the assumption that for any given PE, future ID, and borrow depth, only one future exists. This means that when that future is borrowed, it becomes its own entry in borrowed_futures, with an associated counter called num_borrowers tracking how many borrowers it has. However, this assumption isn't true. You can have, for example, two futures on PE 0 with id 3 and borrow depth 1, each with different num_borrowers values. These futures can then be independently borrowed, and the entry in borrowed_futures on PE 0 with key (3,1) will be the most recently borrowed future with num_borrowers=1. Then, we have the del method called when a future is garbage collected. It sends a call to the parent of the future to delete the future from borrowed_futures on the parent. When running pool_simple.py with two or more PEs, there are two futures on PE 0 with store_id=3 and borrow_depth=1, and each is sent to another PE once. This means that the second future is stored in borrowed_futures with key (3,1), and num_borrowers=1. When the delete comes from PE1, entry (3,1) is deleted because num_borrowers is decremented to 0 and the refcount of the object is 2. But another delete comes from PE 0 for the first future with id 3 and depth 1, and in this delete, key (3,1) doesn't exist, causing a Key Error. My fix is to check if a key in borrowed_futures exists, and the future at that key is different than the current key. If so, I increment the borrow value of the existing future. This way, only one future (the first one sent) is stored in borrowed_futures, and this future will have the correct num_borrowers count. The second future is never really borrowed or tracked, but it's ok because it would have the same state as the first one. |
Todo: implement new scheme for future bookkeeping: Each entry will have a future ID as the key, and each entry will have a single borrow count, and a list of pairs (p_i, c_i), where p_i is the parent PE and c_i is the count borrowed from that parent. The single borrow count is the number of borrowings of this future ID by other PEs, with each separate instance of borrowing counting as one. Self borrowings are decremented during unpacking. During garbage collection, you send a message to each parent with the corresponding count. |
Resolve #263. For the pool examples, this checks if the program is launched with only one PE, and points to the pool documentation if it is (then exits). Also fixes the pool_simple.py example to work (right now, task pools on lists of futures don't work).