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

selection.enter.insertInOrder #744

Closed
wants to merge 4 commits into from
Closed

Conversation

nevir
Copy link

@nevir nevir commented Aug 3, 2012

This spawned from the discussion on #732. Rather than take a functional argument to insert's before argument, a more specialized function seems to make more sense here.

@mbostock
Copy link
Member

mbostock commented Aug 3, 2012

Funny, I implemented this about a year ago, but unfortunately I’ve lost the commit now. It's a tempting feature.

If we include it, it probably makes sense to just have this be the default behavior for enter.append; I don’t think there’s enough weight to merit two separate methods. The performance cost looks negligible.

The big negative is that there is no guarantee (even with this change) that the resulting enter+update selection is in order: that’s because the update selection may not be in order if a key function was used to compute the data join. Hence the need for selection.order, which guarantees that the order of elements will match the data.

I’ll also point out that selection.order is efficient; if the selection is mostly in-order, it does very little work. So typically the performance cost of using selection.order is negligible. Has that been the case in your experience, or if not, could you provide numbers and test cases to guide this discussion?

@nevir
Copy link
Author

nevir commented Aug 3, 2012

I'm seeing a pretty dramatic speed up with this (particularly on iOS, which is where I'm focused): http://jsperf.com/d3-dom-interleaved-insert-performance (edit: I had the benchmark goofed for append/clone - it's closer, but still ~15-20% faster on iOS 6)

I'm not sure I understand the ordering issue you're describing, though. Is that if your key function changes behavior between updates? (all the tests in this pull request use a key func w/ data sorted by key)

@nevir
Copy link
Author

nevir commented Aug 3, 2012

Also, this would probably need a bit more tweaking to be a good append substitute (it's definitely going to be slower for that case since it's currently walking the data & building that lookup array; I could make it a bit lazier about the memoization)

@nevir
Copy link
Author

nevir commented Aug 13, 2012

Bump; I'm using this successfully so far in my own projects - do you want me to address anything else as part of this pull request? (better memoization for the append case?)

@baumandm
Copy link

FYI I'm using this feature successfully as well. It is very useful for updating an HTML tree where nodes are arranged by their document order rather than absolute positioning.

@AlexGalays
Copy link

According to the JsPerf above, isn't the performance gain negligible? Or does it heavily depend on the platform?

@nevir
Copy link
Author

nevir commented Jan 17, 2013

10-15% or so is pretty significant, especially for large sets of nodes. (And I was seeing wins of 20% or so on iOS)

@campersau
Copy link

If I understand it right you want something like this http://bl.ocks.org/campersau/5274509 don't you?

@nevir
Copy link
Author

nevir commented Apr 12, 2013

@campersau interesting approach!

@mbostock
Copy link
Member

mbostock commented Jul 1, 2013

Here’s a simple example to illustrate how insertInOrder won’t guarantee the correct order if the updating elements change order. Let’s say this is our general update pattern:

function update(fruits) {
  var fruit = d3.select("body").selectAll(".fruit")
      .data(fruits, function(d) { return d; });

  fruit.exit().remove();

  fruit.enter().insertInOrder("div")
      .attr("class", "fruit")
      .text(function(d) { return d; });
}

First we enter two fruits:

update(["apple", "orange"]);

The resulting DOM:

<body>
  <div class="fruit">apple</div>
  <div class="fruit">orange</div>
</body>

Now change the data, adding banana and swapping apple and orange:

update(["banana", "orange", "apple"]);

In the resulting DOM, banana is inserted before orange because orange is the next-following element previously in the DOM. However, since insertInOrder only applies to entering elements, orange and apple are not in the correct order with respect to the data:

<body>
  <div class="fruit">apple</div>
  <div class="fruit">banana</div>
  <div class="fruit">orange</div>
</body>

For comparison, here’s the general update using selection.order:

function update(fruits) {
  var fruit = d3.select("body").selectAll(".fruit")
      .data(fruits, function(d) { return d; });

  fruit.exit().remove();

  fruit.enter().append("div")
      .attr("class", "fruit")
      .text(function(d) { return d; });

  fruit.order();
}

This always produces the correct order, even if updating elements get reordered, even if there are no entering elements:

<body>
  <div class="fruit">banana</div>
  <div class="fruit">orange</div>
  <div class="fruit">apple</div>
</body>

Of course, there are many cases where insertInOrder is sufficient; any case where you can guarantee that updating elements are in the same order, and only entering elements need to be inserted into place.

@mbostock
Copy link
Member

mbostock commented Jul 1, 2013

Here’s another way to achieve the same result as insertInOrder by passing a function as the second argument to selection.insert. (It’s currently limited to selections with only a single group, but that could be fixed.)

function update(fruits) {
  var fruit = d3.select("body").selectAll(".fruit")
      .data(fruits, function(d) { return d; });

  fruit.exit().remove();

  fruit.enter().insert("div", sibling)
      .attr("class", "fruit")
      .text(function(d) { return d; });

  // Find the following updating sibling.
  function sibling(d, i) {
    var n = fruit[0].length, e;
    while (++i < n && !(e = fruit[0][i]));
    return e;
  }
}

mbostock added a commit that referenced this pull request Jul 2, 2013
Fixes #744. This implements a default `before` selector such that when entering
elements are inserted (e.g., enter.insert("div")), they are inserted before the
next following sibling in the update selection, if any. This change does not
affect the behavior of append, so as to preserve backwards-compatibility, and
likewise does not affect the behavior of insert on non-enter selections.
@mbostock
Copy link
Member

mbostock commented Jul 2, 2013

Superseded by #1358 which implements this as the default behavior for enter.insert(name).

@mbostock mbostock closed this Jul 2, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants