From 2d66d65874fa0b129b06ff29c310f3f60f0d1be0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Kohlschu=CC=88tter?= Date: Thu, 9 Nov 2023 19:40:26 +0100 Subject: [PATCH] AFSelector: Use single ConcurrentHashMap for registered/selected keys Since we already have a ConcurrentHashMap, let's make use of the value, which now indicates the "selected" state. Introduce MapValueSet, which is a view over elements of the "keysRegistered" map, and precisely only those elements that have a certain value. For each call to select, we increment the expected value, and then set only the actually selected entries to that value, so we don't have to clear the entire map. https://github.com/kohlschutter/junixsocket/issues/145 --- .../org/newsclub/net/unix/AFSelector.java | 33 +- .../org/newsclub/net/unix/MapValueSet.java | 320 ++++++++++++++++++ 2 files changed, 344 insertions(+), 9 deletions(-) create mode 100644 junixsocket-common/src/main/java/org/newsclub/net/unix/MapValueSet.java diff --git a/junixsocket-common/src/main/java/org/newsclub/net/unix/AFSelector.java b/junixsocket-common/src/main/java/org/newsclub/net/unix/AFSelector.java index 28119e133..ba3f01a3f 100644 --- a/junixsocket-common/src/main/java/org/newsclub/net/unix/AFSelector.java +++ b/junixsocket-common/src/main/java/org/newsclub/net/unix/AFSelector.java @@ -33,6 +33,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; final class AFSelector extends AbstractSelector { private final AFPipe selectorPipe; @@ -41,13 +42,15 @@ final class AFSelector extends AbstractSelector { private final ByteBuffer pipeMsgWakeUp = ByteBuffer.allocate(1); private final ByteBuffer pipeMsgReceiveBuffer = ByteBuffer.allocateDirect(256); - private final Map keysRegistered = new ConcurrentHashMap<>(); + private final Map keysRegistered = new ConcurrentHashMap<>(); private final Set keysRegisteredKeySet = keysRegistered.keySet(); private final Set keysRegisteredPublic = Collections.unmodifiableSet( keysRegisteredKeySet); - private final Map selectedKeysSet = new ConcurrentHashMap<>(); - private final Set selectedKeysPublic = new UngrowableSet<>(selectedKeysSet.keySet()); + private final AtomicInteger selectCount = new AtomicInteger(0); + private final MapValueSet selectedKeysSet = + new MapValueSet(keysRegistered, selectCount::get, 0); + private final Set selectedKeysPublic = new UngrowableSet<>(selectedKeysSet); private PollFd pollFd = null; @@ -63,7 +66,7 @@ protected SelectionKey register(AbstractSelectableChannel ch, int ops, Object at AFSelectionKey key = new AFSelectionKey(this, ch, ops, att); synchronized (this) { pollFd = null; - keysRegistered.put(key, Boolean.TRUE); + selectedKeysSet.markRemoved(key); } return key; } @@ -106,12 +109,15 @@ public int select() throws IOException { @SuppressWarnings("PMD.CognitiveComplexity") private int select0(int timeout) throws IOException { PollFd pfd; + + int selectId = updateSelectCount(); + synchronized (this) { if (!isOpen()) { throw new ClosedSelectorException(); } + pfd = pollFd = initPollFd(pollFd); - selectedKeysSet.clear(); } int num; try { @@ -121,7 +127,6 @@ private int select0(int timeout) throws IOException { end(); } synchronized (this) { - selectedKeysSet.clear(); pfd = pollFd; if (pfd != null) { AFSelectionKey[] keys = pfd.keys; @@ -138,7 +143,7 @@ private int select0(int timeout) throws IOException { } if (num > 0) { consumeAllBytesAfterPoll(); - setOpsReady(pfd); // updates keysSelected and numKeysSelected + setOpsReady(pfd, selectId); // updates keysSelected and numKeysSelected } return selectedKeysSet.size(); } @@ -178,14 +183,24 @@ private synchronized void consumeAllBytesAfterPoll() throws IOException { } } - private synchronized void setOpsReady(PollFd pfd) { + private int updateSelectCount() { + int selectId = selectCount.incrementAndGet(); + if (selectId == 0) { + // overflow (unlikely) + selectedKeysSet.markAllRemoved(); + selectId = selectCount.incrementAndGet(); + } + return selectId; + } + + private void setOpsReady(PollFd pfd, int selectId) { if (pfd != null) { for (int i = 1; i < pfd.rops.length; i++) { int rops = pfd.rops[i]; AFSelectionKey key = pfd.keys[i]; key.setOpsReady(rops); if (rops != 0) { - selectedKeysSet.put(key, key); + keysRegistered.computeIfPresent(key, (k, v) -> selectId); } } } diff --git a/junixsocket-common/src/main/java/org/newsclub/net/unix/MapValueSet.java b/junixsocket-common/src/main/java/org/newsclub/net/unix/MapValueSet.java new file mode 100644 index 000000000..11adfeb55 --- /dev/null +++ b/junixsocket-common/src/main/java/org/newsclub/net/unix/MapValueSet.java @@ -0,0 +1,320 @@ +/* + * junixsocket + * + * Copyright 2009-2023 Christian Kohlschütter + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.newsclub.net.unix; + +import java.util.Collection; +import java.util.Collections; +import java.util.ConcurrentModificationException; +import java.util.Iterator; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Objects; +import java.util.Set; +import java.util.function.Supplier; + +import org.eclipse.jdt.annotation.NonNull; + +/** + * A {@link Set} that is a view on the keys of a {@link Map} that have a certain value. + *

+ * The value is controlled by the concrete subclass ({@link #getValue()}). It can, for example, be a + * boolean or a counter, depending on the use case. If the value is equal to a "removed" sentinel + * value. + * + * @param The element type. + * @author Christian Kohlschütter + */ +final class MapValueSet implements Set { + private final Map map; + private final Supplier<@NonNull V> valueSupplier; + private final V removedSentinel; + + @SuppressWarnings("unchecked") + MapValueSet(Map map, Supplier<@NonNull V> valueSupplier, V removedSentinel) { + this.valueSupplier = Objects.requireNonNull(valueSupplier); + this.removedSentinel = removedSentinel; + this.map = (Map) map; + } + + /** + * Marks the given element as "removed"; this may actually add an element to the underlying map. + *

+ * Depending on the "removed" sentinel, the key may be added (if value is non-null but the map + * does not yet contain the key), modified (value is non-null, and the map has a different value + * for the key), or removed (if value is null). + * + * @param elem The element to remove. + */ + public void markRemoved(T elem) { + if (removedSentinel == null) { + map.remove(elem); + } else { + map.put(elem, removedSentinel); + } + } + + /** + * Sets all entries in the backing map to the "removed" sentinel, or removes them all if that + * value is {@code null}. + */ + public void markAllRemoved() { + if (removedSentinel == null) { + map.clear(); + } else { + for (Map.Entry en : map.entrySet()) { + en.setValue(removedSentinel); + } + } + } + + private @NonNull V getValue() { + return Objects.requireNonNull(valueSupplier.get()); + } + + @Override + public int size() { + V val = getValue(); + if (val.equals(removedSentinel)) { + return 0; + } + + int size = 0; + for (Map.Entry en : map.entrySet()) { + if (val.equals(en.getValue())) { + size++; + } + } + return size; + } + + @Override + public boolean isEmpty() { + V val = getValue(); + if (val.equals(removedSentinel)) { + return true; + } + + for (Map.Entry en : map.entrySet()) { + if (val.equals(en.getValue())) { + return false; + } + } + return true; + } + + private boolean isDefinitelyEmpty() { + return getValue().equals(removedSentinel); + } + + @Override + public boolean contains(Object o) { + if (isDefinitelyEmpty()) { + return false; + } + return getValue().equals(map.get(o)); + } + + @Override + public Iterator iterator() { + if (isDefinitelyEmpty()) { + return Collections.emptyIterator(); + } + + Iterator> mapit = map.entrySet().iterator(); + + V val = getValue(); + + return new Iterator() { + Map.Entry nextObj = null; + Map.Entry currentObj = null; + + @Override + public boolean hasNext() { + if (nextObj != null) { + return true; + } + while (mapit.hasNext()) { + Map.Entry en = mapit.next(); + if (val.equals(en.getValue())) { + nextObj = en; + return true; + } + } + return false; + } + + @Override + public T next() { + currentObj = null; + if (nextObj == null) { + if (!hasNext()) { + throw new NoSuchElementException(); + } + } + T next = nextObj.getKey(); + if (val.equals(nextObj.getValue())) { + currentObj = nextObj; + nextObj = null; + return next; + } else { + throw new ConcurrentModificationException(); + } + } + + @Override + public void remove() { + if (currentObj == null) { + throw new IllegalStateException(); + } + markRemoved(currentObj.getKey()); + currentObj = null; + } + }; + } + + @Override + @SuppressWarnings("PMD.OptimizableToArrayCall") + public Object[] toArray() { + return toArray(new Object[size()]); + } + + @SuppressWarnings({"unchecked", "null"}) + @Override + public E[] toArray(E[] a) { + int size = size(); + + if (a.length < size) { + return toArray((E[]) java.lang.reflect.Array.newInstance(a.getClass().getComponentType(), + size)); + } + + int i = 0; + for (T elem : this) { + a[i++] = (E) elem; + } + if (i < a.length) { + a[i] = null; + } + + return a; + } + + /** + * Updates an already-existing entry in the backing map to the current value (obtained via + * {@link #getValue()}), thereby adding it to the set. + * + * @param e The entry to update. + */ + public void update(T e) { + map.computeIfPresent(e, (k, v) -> getValue()); + } + + /** + * Adds an entry to the set, adding it to the backing map if necessary. + */ + @Override + public boolean add(T e) { + if (!map.containsKey(e)) { + map.computeIfAbsent(e, (k) -> getValue()); + return true; + } else if (contains(e)) { + return false; + } else { + update(e); + return true; + } + } + + @SuppressWarnings("unchecked") + @Override + public boolean remove(Object o) { + if (isDefinitelyEmpty() || !map.containsKey(o)) { + return false; + } + + markRemoved((T) o); + return true; + } + + @Override + public boolean containsAll(Collection c) { + if (isDefinitelyEmpty()) { + return c.isEmpty(); + } + for (Object obj : c) { + if (!contains(obj)) { + return false; + } + } + return true; + } + + @Override + public boolean addAll(Collection c) { + boolean changed = false; + for (T elem : c) { + changed |= add(elem); + } + return changed; + } + + @Override + public boolean retainAll(Collection c) { + boolean changed = false; + for (Iterator it = iterator(); it.hasNext();) { + T elem = it.next(); + if (!c.contains(elem)) { + it.remove(); + changed = true; + } + } + return changed; + } + + @Override + public boolean removeAll(Collection c) { + if (isDefinitelyEmpty()) { + return false; + } + boolean changed = false; + for (Object obj : c) { + changed |= remove(obj); + } + return changed; + } + + /** + * Marks all entries in the backing map that are currently considered contained in this set as + * removed; see {@link #markAllRemoved()} for an unoptimized version that affects all keys. + * + * @see #markAllRemoved() + */ + @Override + public void clear() { + V val = getValue(); + if (val.equals(removedSentinel)) { + return; + } + + for (Map.Entry en : map.entrySet()) { + if (val.equals(en.getValue())) { + markRemoved(en.getKey()); + } + } + } +}