vine: priority queue implementation #3919
Conversation
|
To sum up, the manager resets the cursor in two conditions:
And the data structure resets the cursor in four cases:
The manager always resets the cursor to the head, while the data structure always resets to the insert/deletion point - 1. |
There was a problem hiding this comment.
- Is it really necessary to have three iterators in priority_queue.h? Why not just one?
- Add a test
dttools/test/TR_priority_queue.shthat exercises the priority queue independently of taskvine. The data structure must be absolutely bulletproof. - Add a big enormous comment at the beginning of
start_one_taskthat explains why we are iterating over the queue in this way, and where/why the iterator should be reset.
|
@btovar @colinthomas-z80 Could you please take a look at this PR and see if there is anything that is potentially causing any issues? Your suggestions would be greatly helpful as I am modifying the underlying data structure, and there may be areas where I am not very careful. |
|
@btovar please review |
| @param priority The specified priority with the given object. | ||
| @return The idex of data if the push succeeded, -1 on failure. | ||
| */ | ||
| int priority_queue_push_upward(struct priority_queue *pq, void *data, double priority); |
There was a problem hiding this comment.
I don't think we want two versions of push. If the order of two elements with the same priority matters, then they should not have the same priority.
There was a problem hiding this comment.
@btovar This version of push came from a discussion with @dthain, the idea was that when a task is resubmitted given previous resource exhaustion, we want it to be dispatched as soon as possible in order to get more resources allocated to it. For those big tasks, we don't want to change their assigned priorities, but pushing them upward of those with the same priority is likely to get a bigger chance of scheduling earlier.
dttools/src/priority_queue.h
Outdated
| @param index The index of the element to get. | ||
| @return The pointer to the element if any, failure otherwise | ||
| */ | ||
| void *priority_queue_get_element(struct priority_queue *pq, int index); |
There was a problem hiding this comment.
peek_at? Also, allow index 0 and do a +1 internally. Otherwise the abstraction is leaking.
Proposed Changes
Give an overall description of the changes, along with the context and motivation.
Mention relevant issues and pull requests as needed.
Merge Checklist
The following items must be completed before PRs can be merge.
Check these off to verify you have completed all steps.
make testRun local tests prior to pushing.make formatFormat source code to comply with lint policies. Note that some lint errors can only be resolved manually (e.g., Python)make lintRun lint on source code prior to pushing.