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

Implement append for BinaryHeap #32526

Closed
apasel422 opened this issue Mar 27, 2016 · 4 comments
Closed

Implement append for BinaryHeap #32526

apasel422 opened this issue Mar 27, 2016 · 4 comments
Labels
A-collections Area: `std::collection` B-unstable Blocker: Implemented in the nightly compiler and unstable. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@apasel422
Copy link
Contributor

This can be done in O(n + m) time by appending the underlying vectors and then using the build heap algorithm in the From<Vec<T>> impl. Note that in some circumstances this will perform worse than repeatedly pushing (as is done in h1.extend(h2)), because the time complexity of that approach is O(m log (m + n)). In both approaches, append should ensure that the larger of the two heaps acts as the "destination" of the merge in order to move as few elements as possible.

append should use a heuristic to determine which approach to take. This should be based on the sizes of the two heaps, but may also incorporate the vectors' capacities.

Pseudo-code:

use std::mem::swap;

fn shouldMergeWithBuildHeap<T>(v1: &Vec<T>, v2: &Vec<T>) -> bool { ... }

impl<T: Ord> BinaryHeap<T> {
    pub fn append(&self, other: &mut Self) {
        if other.is_empty() {
            return;
        }

        if self.is_empty() {
            swap(self, other);
            return;
        }

        if self.len() < other.len() {
            swap(self, other);
        }

        if shouldMergeWithBuildHeap(&self.data, &other.data) {
            self.data.append(&mut other.data.append);
            self.build_heap();
        } else {
            self.extend(other.drain());
        }
    }

    fn build_heap(&mut self) {
        let mut n = self.len() / 2;
        while n > 0 {
            n -= 1;
            self.sift_down(n);
        }
    }
}

For now, a simple version that just extends the smaller one into the larger one could suffice:

impl<T: Ord> BinaryHeap<T> {
    pub fn append(&self, other: &mut Self) {
        if other.is_empty() {
            return;
        }

        if self.is_empty() {
            swap(self, other);
            return;
        }

        if self.len() < other.len() {
            swap(self, other);
        }

        self.extend(other.drain());
    }
}
@apasel422 apasel422 added the A-collections Area: `std::collection` label Mar 27, 2016
@xosmig
Copy link
Contributor

xosmig commented Apr 14, 2016

I'd like to perform it.

@apasel422
Copy link
Contributor Author

@xosmig Cool. Feel free to submit a PR!

@apasel422 apasel422 added the B-unstable Blocker: Implemented in the nightly compiler and unstable. label Apr 17, 2016
@alexcrichton alexcrichton added I-nominated T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jun 9, 2016
@alexcrichton
Copy link
Member

🔔 This issue is now entering its final comment period for stabilization 🔔

The libs team was unfortunately a little late on stabilizations this cycle, but we're thinking of probably doing a backport of stabilizations partway through the beta cycle. This API also follows existing conventions so should be an easy stabilization!

@alexcrichton alexcrichton added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed I-nominated labels Jun 20, 2016
@alexcrichton
Copy link
Member

The libs team discussed this issue during triage yesterday and the decision was to stabilize. We realize that the FCP for this issue was pretty short, however, so please comment with any objections you might have! We're very willing to backport an un-stabilization for the few APIs we have this cycle.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 3, 2016
Although the set of APIs being stabilized this release is relatively small, the
trains keep going! Listed below are the APIs in the standard library which have
either transitioned from unstable to stable or those from unstable to
deprecated.

Stable

* `BTreeMap::{append, split_off}`
* `BTreeSet::{append, split_off}`
* `Cell::get_mut`
* `RefCell::get_mut`
* `BinaryHeap::append`
* `{f32, f64}::{to_degrees, to_radians}` - libcore stabilizations mirroring past
  libstd stabilizations
* `Iterator::sum`
* `Iterator::product`

Deprecated

* `{f32, f64}::next_after`
* `{f32, f64}::integer_decode`
* `{f32, f64}::ldexp`
* `{f32, f64}::frexp`
* `num::One`
* `num::Zero`

Added APIs (all unstable)

* `iter::Sum`
* `iter::Product`
* `iter::Step` - a few methods were added to accomodate deprecation of One/Zero

Removed APIs

* `From<Range<T>> for RangeInclusive<T>` - everything about `RangeInclusive` is
  unstable

Closes rust-lang#27739
Closes rust-lang#27752
Closes rust-lang#32526
Closes rust-lang#33444
Closes rust-lang#34152
cc rust-lang#34529 (new tracking issue)
bors added a commit that referenced this issue Jul 3, 2016
std: Stabilize APIs for the 1.11.0 release

Although the set of APIs being stabilized this release is relatively small, the
trains keep going! Listed below are the APIs in the standard library which have
either transitioned from unstable to stable or those from unstable to
deprecated.

Stable

* `BTreeMap::{append, split_off}`
* `BTreeSet::{append, split_off}`
* `Cell::get_mut`
* `RefCell::get_mut`
* `BinaryHeap::append`
* `{f32, f64}::{to_degrees, to_radians}` - libcore stabilizations mirroring past
  libstd stabilizations
* `Iterator::sum`
* `Iterator::product`

Deprecated

* `{f32, f64}::next_after`
* `{f32, f64}::integer_decode`
* `{f32, f64}::ldexp`
* `{f32, f64}::frexp`
* `num::One`
* `num::Zero`

Added APIs (all unstable)

* `iter::Sum`
* `iter::Product`
* `iter::Step` - a few methods were added to accomodate deprecation of One/Zero

Removed APIs

* `From<Range<T>> for RangeInclusive<T>` - everything about `RangeInclusive` is
  unstable

Closes #27739
Closes #27752
Closes #32526
Closes #33444
Closes #34152
cc #34529 (new tracking issue)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-collections Area: `std::collection` B-unstable Blocker: Implemented in the nightly compiler and unstable. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants