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

mutable.LinkedHashSet remove() performance is O(n), could/should be O(1) #4639

Closed
scabug opened this issue May 24, 2011 · 3 comments
Closed

Comments

@scabug
Copy link

scabug commented May 24, 2011

Current implementation of mutable.LinkedHashSet uses a ListBuffer as a companion of a standard HashSet. Consequently, a call to remove() actually calls both HashSet and ListBuffer's remove() methods, ListBuffer's being in O(n ).

An full O(1) implementation is expected and doable (for example, by storing double-linked cells in the HashSet, but something even better might exists).

I have not checked yet whether the immutable version is also affected.

@scabug
Copy link
Author

scabug commented May 24, 2011

Imported From: https://issues.scala-lang.org/browse/SI-4639?orig=1
Reporter: Julien Vion (scand1sk)
Assignee: @pavelpavlov
Affected Versions: 2.9.0

@scabug
Copy link
Author

scabug commented May 24, 2011

Julien Vion (scand1sk) said (edited on May 24, 2011 1:43:48 PM UTC):
Here is a tentative implementation, which can probably be made much better (by avoiding the instantiation of a wrapper object on every Set operation, for instance).

{code:title=MyLinkedHashSet.scala}

package scala.collection
package mutable

import collection.generic._

class MyLinkedHashSet[A] extends Set[A]
with GenericSetTemplate[A, MyLinkedHashSet]
with SetLike[A, MyLinkedHashSet[A]] {
override def companion: GenericCompanion[MyLinkedHashSet] = MyLinkedHashSet

override def size = hashSet.size

def contains(elem: A): Boolean = hashSet.contains(new Cell(elem))

def +=(elem: A): this.type = { add(elem); this }
def -=(elem: A): this.type = { remove(elem); this }

private val hashSet = new HashSet[Cell[A]]
private var firstCell: Cell[A] = null
private var lastCell: Cell[A] = null

override def add(elem: A): Boolean = {
val c = new Cell(elem)
if (hashSet.add(c)) {
if (lastCell == null) {
assert(firstCell == null)
firstCell = c;
} else {
c.prev = lastCell;
lastCell.next = c;
}
lastCell = c
true
} else false
}

override def remove(elem: A): Boolean = {
val c = new Cell(elem)
if (hashSet.remove(c)) {
if (c.prev == null) {
firstCell = c.next
} else {
c.prev.next = c.next
}
if (c.next == null) {
lastCell = c.prev
} else {
c.next.prev = c.prev
}
true
} else false
}

override def clear() {
hashSet.clear()
firstCell = null
lastCell = null
}

def iterator = new Iterator[A] {
var current = firstCell
def hasNext = current != null
def next = {
val ret = current;
current = current.next;
ret.content;
}
}

override def foreach[U](f: A => U) = iterator foreach f

}

class Cell[A](val content: A) {
var next: Cell[A] = null;
var prev: Cell[A] = null;

override val hashCode = content.hashCode
override def equals(obj: Any) = obj match {
case c: Cell[A] => content == c.content
case _ => false
}
}

/**

  • $factoryInfo
  • @define Coll MyLinkedHashSet
  • @define coll linked hash set
    */
    object MyLinkedHashSet extends MutableSetFactory[MyLinkedHashSet] {
    implicit def canBuildFrom[A]: CanBuildFrom[Coll, A, MyLinkedHashSet[A]] = setCanBuildFrom[A]
    override def empty[A]: MyLinkedHashSet[A] = new MyLinkedHashSet[A]
    }

{code}

@scabug
Copy link
Author

scabug commented Sep 18, 2012

@pavelpavlov said:
the same as #5767

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant