Skip to content

Deprecate seqSpine#95

Closed
treeowl wants to merge 1 commit intolspitzner:masterfrom
treeowl:deprecate-seqSpine
Closed

Deprecate seqSpine#95
treeowl wants to merge 1 commit intolspitzner:masterfrom
treeowl:deprecate-seqSpine

Conversation

@treeowl
Copy link
Copy Markdown
Collaborator

@treeowl treeowl commented Mar 26, 2023

The seqSpine function was useful back in the day when all sorts of operations were written in a way that allowed thunks to accumulate along the spine. Now, we carefully force as much of the spine as the amortized bounds allow, which eliminates this problem in general. The only operations that still accumulate thunks along the spine are monotonic maps, and forcing the spines of those just leads to the creation of more thunks at the leaves—it's not really useful.

Comment thread src/Data/PQueue/Max.hs Outdated
Comment thread src/Data/PQueue/Prio/Max/Internals.hs Outdated
@konsumlamm
Copy link
Copy Markdown
Collaborator

Can you elaborate why mapU/mapKeysMonotonic don't benefit from seqSpine? Is it because all the work is accumulated in the elements?

@treeowl treeowl force-pushed the deprecate-seqSpine branch 2 times, most recently from 47592dc to e17bdd5 Compare March 26, 2023 20:07
@treeowl
Copy link
Copy Markdown
Collaborator Author

treeowl commented Mar 26, 2023

Can you elaborate why mapU/mapKeysMonotonic don't benefit from seqSpine? Is it because all the work is accumulated in the elements?

Exactly. I've updated the documentation to explain that, and I've also added references to more functions with similar issues in he Prio case.

The `seqSpine` function was useful back in the day when all sorts of
operations were written in a way that allowed thunks to accumulate along
the spine. Now, we carefully force as much of the spine as the amortized
bounds allow, which eliminates this problem in general. The only
operations that still accumulate thunks along the spine are monotonic
maps, and forcing the spines of those just leads to the creation of more
thunks at the leaves—it's not really useful.
@treeowl treeowl force-pushed the deprecate-seqSpine branch from e17bdd5 to e3c2b96 Compare March 26, 2023 20:09
@treeowl
Copy link
Copy Markdown
Collaborator Author

treeowl commented Mar 26, 2023

Mapping over a queue gives you (a thunk for) the first element, and a thunk for the rest of the queue. As you push your way down the spine, successive vertebrae each produce larger and larger numbers of thunks.

-- | \(O(\log n)\). @seqSpine q r@ forces the spine of @q@ and returns @r@.
--
-- Note: The spine of a 'MaxPQueue' is stored somewhat lazily. Most operations
-- Note: The spine of a 'MinPQueue' is stored somewhat lazily. Most operations
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This was correct, wasn't it?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ugh. Whoops.

@treeowl
Copy link
Copy Markdown
Collaborator Author

treeowl commented Mar 26, 2023

Hahaha. This entire PR is based on me getting confused. Whooops. There really was a (minor) reason for these functions to persist, and it actually was only the functions that were in the documentation. I should explain more about why that is somewhere...

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.

2 participants