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

worklist changes for partr (1/3) #30806

Merged
merged 5 commits into from
Mar 19, 2019
Merged

worklist changes for partr (1/3) #30806

merged 5 commits into from
Mar 19, 2019

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Jan 22, 2019

This PR attempts to implement most of the representation changes in kp/partr, while avoiding actually bringing in the multiq code (yet). These changes are actually mostly independent of that change (e.g we could instead adapt that PR to use arrays, then decide whether to make this change later; or we could chose to do this PR, then never merge partr). But these changes that are being proposed over there seem sensible to me as a stepping stone.

TODO:

  • Fix name of list_deletefirst!
  • Is it worthwhile to try to import more of the sticky-queue logic from partr? We're adding most of the ability to manage it here (and some of the overhead), but likely still don't fully support using Tasks on the worker threads here—can that wait for the partr PR to finish it, or do we want to move the bits from there that would be needed to make it work (prior to merging partr to make it efficient)?

@vtjnash vtjnash changed the title [wip] worklist changes for partr [wip] worklist changes for partr (1/3) Jan 25, 2019
while head_next !== val
head = head_next
head_next = head.next
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if head_next === nothing ... ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first line of this function should guarantee that val is found.

@kpamnany
Copy link
Contributor

So this is basically moving task-related code from event.jl into task.jl and adding InvasiveLinkedList{T}? I'm not quite sure of the motivation for this but it looks okay to me barring the single problem I commented on with the while loop.

@mbauman mbauman added the multithreading Base.Threads and related functionality label Mar 5, 2019
@JeffBezanson
Copy link
Member

Part 2 merged into this branch.

@JeffBezanson JeffBezanson changed the title [wip] worklist changes for partr (1/3) worklist changes for partr (1/3) Mar 19, 2019
@JeffBezanson JeffBezanson merged commit 41c3369 into master Mar 19, 2019
@vchuravy vchuravy deleted the jn/task-ll branch March 19, 2019 02:21
Keno pushed a commit that referenced this pull request Jun 5, 2024
worklist changes for partr (1/3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multithreading Base.Threads and related functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants