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.

Sign up for GitHub

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

Optimize HashPMap.get and other operations: do not create unnecessary iterator instances #41

Closed
wants to merge 4 commits into from
Closed

Optimize HashPMap.get and other operations: do not create unnecessary iterator instances #41

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b3b975
Use ConsPStack instead of PSequence in HashPMap internals
udalov Aug 15, 2016
8cb12ae
Do not create unnecessary ConsPStack iterators in HashPMap
udalov Aug 15, 2016
059cae6
Extract anonymous classes to inner
udalov Aug 15, 2016
ce46586
Fix infinite loop in HashPMap.keyIndexIn
udalov Feb 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 23 additions & 16 deletions src/main/java/org/pcollections/AmortizedPQueue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,22 +53,29 @@ private AmortizedPQueue(PStack<E> front, PStack<E> back) {
/* Worst-case O(n) */
@Override
public Iterator<E> iterator() {
return new Iterator<E>() {
private PQueue<E> queue = AmortizedPQueue.this;
public boolean hasNext() {
return queue.size() > 0;
}

public E next() {
E e = queue.peek();
queue = queue.minus();
return e;
}

public void remove() {
throw new UnsupportedOperationException();
}
};
return new Itr<E>(this);
}

private static class Itr<E> implements Iterator<E> {
private AmortizedPQueue<E> queue;

Itr(AmortizedPQueue<E> queue) {
this.queue = queue;
}

public boolean hasNext() {
return queue.size() > 0;
}

public E next() {
E e = queue.peek();
queue = queue.minus();
return e;
}

public void remove() {
throw new UnsupportedOperationException();
}
}

/* Worst-case O(1) */
Expand Down
68 changes: 38 additions & 30 deletions src/main/java/org/pcollections/ConsPStack.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ private static <E> ConsPStack<E> from(final Iterator<? extends E> i) {


//// PRIVATE CONSTRUCTORS ////
private final E first; private final ConsPStack<E> rest;
final E first; final ConsPStack<E> rest;
private final int size;
// not externally instantiable (or subclassable):
private ConsPStack() { // EMPTY constructor
Expand All @@ -84,38 +84,46 @@ public int size() {
public ListIterator<E> listIterator(final int index) {
if(index<0 || index>size) throw new IndexOutOfBoundsException();

return new ListIterator<E>() {
int i = index;
ConsPStack<E> next = subList(index);
return new Itr(index);
}

public boolean hasNext() {
return next.size>0; }
public boolean hasPrevious() {
return i>0; }
public int nextIndex() {
return index; }
public int previousIndex() {
return index-1; }
public E next() {
E e = next.first;
next = next.rest;
return e;
}
public E previous() {
System.err.println("ConsPStack.listIterator().previous() is inefficient, don't use it!");
next = subList(index-1); // go from beginning...
return next.first;
}
private class Itr implements ListIterator<E> {
private final int index;
int i;
ConsPStack<E> next;

public void add(final E o) {
throw new UnsupportedOperationException(); }
public void remove() {
throw new UnsupportedOperationException(); }
public void set(final E o) {
throw new UnsupportedOperationException(); }
};
}
Itr(int index) {
this.index = index;
i = index;
next = subList(index);
}

public boolean hasNext() {
return next.size>0; }
public boolean hasPrevious() {
return i>0; }
public int nextIndex() {
return index; }
public int previousIndex() {
return index -1; }
public E next() {
E e = next.first;
next = next.rest;
return e;
}
public E previous() {
System.err.println("ConsPStack.listIterator().previous() is inefficient, don't use it!");
next = subList(index -1); // go from beginning...
return next.first;
}

public void add(final E o) {
throw new UnsupportedOperationException(); }
public void remove() {
throw new UnsupportedOperationException(); }
public void set(final E o) {
throw new UnsupportedOperationException(); }
}

//// OVERRIDDEN METHODS FROM AbstractSequentialList ////
@Override
Expand Down
72 changes: 39 additions & 33 deletions src/main/java/org/pcollections/HashPMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,16 @@ public final class HashPMap<K,V> extends AbstractMap<K,V> implements PMap<K,V> {
* @return a map backed by an empty version of intMap,
* i.e. backed by intMap.minusAll(intMap.keySet())
*/
@SuppressWarnings("unchecked")
public static <K,V> HashPMap<K,V> empty(final PMap<Integer,PSequence<Entry<K,V>>> intMap) {
return new HashPMap<K,V>(intMap.minusAll(intMap.keySet()), 0); }
return new HashPMap<K,V>((PMap) intMap.minusAll(intMap.keySet()), 0); }


//// PRIVATE CONSTRUCTORS ////
private final PMap<Integer,PSequence<Entry<K,V>>> intMap;
private final PMap<Integer,ConsPStack<Entry<K,V>>> intMap;
private final int size;
// not externally instantiable (or subclassable):
private HashPMap(final PMap<Integer,PSequence<Entry<K,V>>> intMap, final int size) {
private HashPMap(final PMap<Integer,ConsPStack<Entry<K,V>>> intMap, final int size) {
this.intMap = intMap; this.size = size; }


Expand All @@ -53,27 +54,28 @@ private HashPMap(final PMap<Integer,PSequence<Entry<K,V>>> intMap, final int siz
@Override
public Set<Entry<K,V>> entrySet() {
if(entrySet==null)
entrySet = new AbstractSet<Entry<K,V>>() {
// REQUIRED METHODS OF AbstractSet //
@Override
public int size() {
return size; }
@Override
public Iterator<Entry<K,V>> iterator() {
return new SequenceIterator<Entry<K,V>>(intMap.values().iterator()); }
// OVERRIDDEN METHODS OF AbstractSet //
@Override
public boolean contains(final Object e) {
if(!(e instanceof Entry))
return false;
V value = get(((Entry<?,?>)e).getKey());
return value!=null && value.equals(((Entry<?,?>)e).getValue());
}
};
entrySet = new EntrySet();
return entrySet;
}


private class EntrySet extends AbstractSet<Entry<K,V>> {
// REQUIRED METHODS OF AbstractSet //
@Override
public int size() {
return size; }
@Override
public Iterator<Entry<K,V>> iterator() {
return new SequenceIterator<Entry<K,V>>(intMap.values().iterator()); }
// OVERRIDDEN METHODS OF AbstractSet //
@Override
public boolean contains(final Object e) {
if(!(e instanceof Map.Entry))
return false;
V value = get(((Entry<?,?>)e).getKey());
return value!=null && value.equals(((Entry<?,?>)e).getValue());
}
}

//// OVERRIDDEN METHODS FROM AbstractMap ////
@Override
public int size() {
Expand All @@ -85,10 +87,13 @@ public boolean containsKey(final Object key) {

@Override
public V get(final Object key) {
PSequence<Entry<K,V>> entries = getEntries(key.hashCode());
for(Entry<K,V> entry : entries)
if(entry.getKey().equals(key))
ConsPStack<Entry<K,V>> entries = getEntries(key.hashCode());
while(entries != null && !entries.isEmpty()) {
Entry<K,V> entry = entries.first;
if (entry.getKey().equals(key))
return entry.getValue();
entries = entries.rest;
}
return null;
}

Expand All @@ -109,7 +114,7 @@ public HashPMap<K,V> minusAll(final Collection<?> keys) {
}

public HashPMap<K,V> plus(final K key, final V value) {
PSequence<Entry<K,V>> entries = getEntries(key.hashCode());
ConsPStack<Entry<K,V>> entries = getEntries(key.hashCode());
int size0 = entries.size(),
i = keyIndexIn(entries, key);
if(i!=-1) entries = entries.minus(i);
Expand All @@ -119,7 +124,7 @@ public HashPMap<K,V> plus(final K key, final V value) {
}

public HashPMap<K,V> minus(final Object key) {
PSequence<Entry<K,V>> entries = getEntries(key.hashCode());
ConsPStack<Entry<K,V>> entries = getEntries(key.hashCode());
int i = keyIndexIn(entries, key);
if(i==-1) // key not in this
return this;
Expand All @@ -134,28 +139,29 @@ public HashPMap<K,V> minus(final Object key) {


//// PRIVATE UTILITIES ////
private PSequence<Entry<K,V>> getEntries(final int hash) {
PSequence<Entry<K,V>> entries = intMap.get(hash);
private ConsPStack<Entry<K,V>> getEntries(final int hash) {
ConsPStack<Entry<K,V>> entries = intMap.get(hash);
if(entries==null) return ConsPStack.empty();
return entries;
}


//// PRIVATE STATIC UTILITIES ////
private static <K,V> int keyIndexIn(final PSequence<Entry<K,V>> entries, final Object key) {
private static <K,V> int keyIndexIn(final ConsPStack<Entry<K,V>> entries, final Object key) {
int i=0;
for(Entry<K,V> entry : entries) {
while(entries != null && !entries.isEmpty()) {
Entry<K,V> entry = entries.first;
udalov marked this conversation as resolved.
Show resolved Hide resolved
if(entry.getKey().equals(key))
return i;
i++;
}
return -1;
}

static class SequenceIterator<E> implements Iterator<E> {
private final Iterator<PSequence<E>> i;
private static class SequenceIterator<E> implements Iterator<E> {
private final Iterator<ConsPStack<E>> i;
private PSequence<E> seq = ConsPStack.empty();
SequenceIterator(Iterator<PSequence<E>> i) {
SequenceIterator(Iterator<ConsPStack<E>> i) {
this.i = i; }

public boolean hasNext() {
Expand Down
37 changes: 19 additions & 18 deletions src/main/java/org/pcollections/IntTreePMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,27 +97,28 @@ IntTreePMap<V> withKeysChangedBelow(final int key, final int delta) {
@Override
public Set<Entry<Integer,V>> entrySet() {
if(entrySet==null)
entrySet = new AbstractSet<Entry<Integer,V>>() {
// REQUIRED METHODS OF AbstractSet //
@Override
public int size() { // same as Map
return IntTreePMap.this.size(); }
@Override
public Iterator<Entry<Integer,V>> iterator() {
return root.iterator(); }
// OVERRIDDEN METHODS OF AbstractSet //
@Override
public boolean contains(final Object e) {
if(!(e instanceof Entry))
return false;
V value = get(((Entry<?,?>)e).getKey());
return value!=null && value.equals(((Entry<?,?>)e).getValue());
}
};
entrySet = new EntrySet();
return entrySet;
}


private class EntrySet extends AbstractSet<Entry<Integer,V>> {
// REQUIRED METHODS OF AbstractSet //
@Override
public int size() { // same as Map
return IntTreePMap.this.size(); }
@Override
public Iterator<Entry<Integer,V>> iterator() {
return root.iterator(); }
// OVERRIDDEN METHODS OF AbstractSet //
@Override
public boolean contains(final Object e) {
if(!(e instanceof Map.Entry))
return false;
V value = get(((Entry<?,?>)e).getKey());
return value!=null && value.equals(((Entry<?,?>)e).getValue());
}
}

//// OVERRIDDEN METHODS FROM AbstractMap ////
@Override
public int size() {
Expand Down
37 changes: 20 additions & 17 deletions src/main/java/org/pcollections/MapPBag.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.util.AbstractCollection;
import java.util.Collection;
import java.util.Iterator;
import java.util.Map;
import java.util.Map.Entry;


Expand Down Expand Up @@ -43,25 +44,27 @@ public int size() {
return size; }
@Override
public Iterator<E> iterator() {
final Iterator<Entry<E,Integer>> i = map.entrySet().iterator();
return new Iterator<E>() {
private E e; private int n=0;
public boolean hasNext() {
return n>0 || i.hasNext(); }
public E next() {
if(n==0) { // finished with current element
Entry<E,Integer> entry = i.next();
e = entry.getKey();
n = entry.getValue();
}
n--;
return e;
return new Itr();
}

private class Itr implements Iterator<E> {
private final Iterator<Entry<E, Integer>> i = map.entrySet().iterator();
private E e;
private int n;
public boolean hasNext() {
return n>0 || i.hasNext(); }
public E next() {
if(n==0) { // finished with current element
Entry<E,Integer> entry = i.next();
e = entry.getKey();
n = entry.getValue();
}
public void remove() {
throw new UnsupportedOperationException(); }
};
n--;
return e;
}
public void remove() {
throw new UnsupportedOperationException(); }
}


//// OVERRIDDEN METHODS OF AbstractCollection ////
@Override
Expand Down