Skip to content

Commit

Permalink
Improve efficiency of bulk removals in transformed lists.
Browse files Browse the repository at this point in the history
Override removeRange(int,int) for Lists.TransformingSequentialList and
Lists.TransformingRandomAccessList so that source list has opportunity
to do more efficient deletions. Call fromList.subList(from, to).clear()
instead of removing with iterator.

Fixes #5762
Fixes #6087

RELNOTES=n/a
PiperOrigin-RevId: 542355023
  • Loading branch information
hwaite authored and Google Java Core Libraries committed Jun 21, 2023
1 parent 3286f01 commit 8965414
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 8 deletions.
14 changes: 10 additions & 4 deletions android/guava/src/com/google/common/collect/Lists.java
Original file line number Diff line number Diff line change
Expand Up @@ -555,8 +555,8 @@ private static class TransformingSequentialList<
* can be overkill. That's why we forward this call directly to the backing list.
*/
@Override
public void clear() {
fromList.clear();
protected void removeRange(int fromIndex, int toIndex) {
fromList.subList(fromIndex, toIndex).clear();
}

@Override
Expand Down Expand Up @@ -596,9 +596,13 @@ private static class TransformingRandomAccessList<
this.function = checkNotNull(function);
}

/**
* The default implementation inherited is based on iteration and removal of each element which
* can be overkill. That's why we forward this call directly to the backing list.
*/
@Override
public void clear() {
fromList.clear();
protected void removeRange(int fromIndex, int toIndex) {
fromList.subList(fromIndex, toIndex).clear();
}

@Override
Expand All @@ -622,6 +626,8 @@ T transform(F from) {
};
}

// TODO: cpovirk - Why override `isEmpty` here but not in TransformingSequentialList?

@Override
public boolean isEmpty() {
return fromList.isEmpty();
Expand Down
14 changes: 10 additions & 4 deletions guava/src/com/google/common/collect/Lists.java
Original file line number Diff line number Diff line change
Expand Up @@ -556,8 +556,8 @@ private static class TransformingSequentialList<
* can be overkill. That's why we forward this call directly to the backing list.
*/
@Override
public void clear() {
fromList.clear();
protected void removeRange(int fromIndex, int toIndex) {
fromList.subList(fromIndex, toIndex).clear();
}

@Override
Expand Down Expand Up @@ -603,9 +603,13 @@ private static class TransformingRandomAccessList<
this.function = checkNotNull(function);
}

/**
* The default implementation inherited is based on iteration and removal of each element which
* can be overkill. That's why we forward this call directly to the backing list.
*/
@Override
public void clear() {
fromList.clear();
protected void removeRange(int fromIndex, int toIndex) {
fromList.subList(fromIndex, toIndex).clear();
}

@Override
Expand All @@ -629,6 +633,8 @@ T transform(F from) {
};
}

// TODO: cpovirk - Why override `isEmpty` here but not in TransformingSequentialList?

@Override
public boolean isEmpty() {
return fromList.isEmpty();
Expand Down

0 comments on commit 8965414

Please sign in to comment.