Add UserThreadMappedPersistableEnvelope for greater explicitness

Make the default toPersistableMessage() method of PersistableEnvelope
simply delegate to Proto.toProtoMessage for speed, so that stores can
explicitly implement (Threaded|UserThreadMapped)PersistableEnvelope if
they actually need concurrency control.

As part of this, make PeerList implement PersistableEnvelope directly
instead of extending PersistableList, as it is non-critical & cloned on
the user thread prior to storage anyway, so doesn't need be thread-safe.
In this way, only PaymentAccountList & small DAO-related stores extend
PersistableList, so they can all be made user-thread-mapped.

After this change, the only concrete store classes not implementing
(Threaded|UserThreadMapped)PersistableEnvelope are:

  AccountAgeWitness, BlindVotePayload, ProposalPayload, SignedWitness,
  TradeStatistics2, NavigationPath & PeerList

The first five appear to erroneously implement PersistableEnvelope and
can be cleaned up in a separate commit. The last two are non-critical.

(Make NavigationPath.path an immutable list, for slightly better thread
safety anyway - that way it will never be observed half-constructed.)
This commit is contained in:
Steven Barclay 2020-03-10 11:44:23 +08:00
parent 9e9fc6ab57
commit 91fa07343c
No known key found for this signature in database
GPG key ID: 9FED6BF1176D500B
11 changed files with 76 additions and 32 deletions

View file

@ -21,7 +21,6 @@ import bisq.common.util.CollectionUtils;
import com.google.protobuf.Message;
import java.util.ArrayList;
import java.util.List;
import lombok.AllArgsConstructor;
@ -36,7 +35,7 @@ import lombok.Setter;
@Getter
@Setter
public class NavigationPath implements PersistableEnvelope {
private List<String> path = new ArrayList<>();
private List<String> path = List.of();
@Override
public Message toProtoMessage() {
@ -46,6 +45,6 @@ public class NavigationPath implements PersistableEnvelope {
}
public static NavigationPath fromProto(protobuf.NavigationPath proto) {
return new NavigationPath(new ArrayList<>(proto.getPathList()));
return new NavigationPath(List.copyOf(proto.getPathList()));
}
}

View file

@ -18,23 +18,15 @@
package bisq.common.proto.persistable;
import bisq.common.Envelope;
import bisq.common.UserThread;
import com.google.protobuf.Message;
import com.google.common.util.concurrent.Futures;
import java.util.concurrent.FutureTask;
/**
* Interface for the outside envelope object persisted to disk.
*/
public interface PersistableEnvelope extends Envelope {
default Message toPersistableMessage() {
FutureTask<Message> toProtoOnUserThread = new FutureTask<>(this::toProtoMessage);
UserThread.execute(toProtoOnUserThread);
//noinspection UnstableApiUsage
return Futures.getUnchecked(toProtoOnUserThread);
return toProtoMessage();
}
}

View file

@ -31,7 +31,7 @@ import lombok.Setter;
import lombok.experimental.Delegate;
@EqualsAndHashCode
public class PersistableList<T extends PersistablePayload> implements PersistableEnvelope, Iterable<T> {
public class PersistableList<T extends PersistablePayload> implements UserThreadMappedPersistableEnvelope, Iterable<T> {
@Delegate(excludes = ExcludesDelegateMethods.class)
@Getter
@Setter

View file

@ -0,0 +1,45 @@
/*
* This file is part of Bisq.
*
* Bisq is free software: you can redistribute it and/or modify it
* under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or (at
* your option) any later version.
*
* Bisq is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
* License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with Bisq. If not, see <http://www.gnu.org/licenses/>.
*/
package bisq.common.proto.persistable;
import bisq.common.UserThread;
import com.google.protobuf.Message;
import com.google.common.util.concurrent.Futures;
import java.util.concurrent.FutureTask;
/**
* Interface for the outer envelope object persisted to disk, where its serialization
* during persistence is forced to take place on the user thread.
* <p>
* To avoid jitter, this should be only be used for small, safely critical stores. Larger
* or frequently written stores should either implement {@link PersistableEnvelope}
* directly (where thread-safety isn't needed) or use {@link ThreadedPersistableEnvelope}.
*/
public interface UserThreadMappedPersistableEnvelope extends PersistableEnvelope {
@Override
default Message toPersistableMessage() {
FutureTask<Message> toProtoOnUserThread = new FutureTask<>(this::toProtoMessage);
UserThread.execute(toProtoOnUserThread);
//noinspection UnstableApiUsage
return Futures.getUnchecked(toProtoOnUserThread);
}
}

View file

@ -17,8 +17,8 @@
package bisq.core.btc.model;
import bisq.common.proto.persistable.PersistableEnvelope;
import bisq.common.proto.persistable.PersistedDataHost;
import bisq.common.proto.persistable.UserThreadMappedPersistableEnvelope;
import bisq.common.storage.Storage;
import com.google.protobuf.Message;
@ -44,7 +44,7 @@ import lombok.extern.slf4j.Slf4j;
*/
@ToString
@Slf4j
public final class AddressEntryList implements PersistableEnvelope, PersistedDataHost {
public final class AddressEntryList implements UserThreadMappedPersistableEnvelope, PersistedDataHost {
transient private Storage<AddressEntryList> storage;
transient private Wallet wallet;
@Getter

View file

@ -19,6 +19,7 @@ package bisq.core.support.dispute;
import bisq.common.proto.persistable.PersistableEnvelope;
import bisq.common.proto.persistable.PersistedDataHost;
import bisq.common.proto.persistable.UserThreadMappedPersistableEnvelope;
import bisq.common.storage.Storage;
import javafx.collections.FXCollections;
@ -39,7 +40,7 @@ import lombok.extern.slf4j.Slf4j;
* Calls to the List are delegated because this class intercepts the add/remove calls so changes
* can be saved to disc.
*/
public abstract class DisputeList<T extends PersistableEnvelope> implements PersistableEnvelope, PersistedDataHost {
public abstract class DisputeList<T extends PersistableEnvelope> implements UserThreadMappedPersistableEnvelope, PersistedDataHost {
transient protected final Storage<T> storage;
@Getter

View file

@ -23,7 +23,7 @@ import bisq.core.proto.CoreProtoResolver;
import bisq.common.proto.ProtoUtil;
import bisq.common.proto.ProtobufferRuntimeException;
import bisq.common.proto.persistable.PersistableEnvelope;
import bisq.common.proto.persistable.UserThreadMappedPersistableEnvelope;
import bisq.common.storage.Storage;
import com.google.protobuf.Message;
@ -41,7 +41,7 @@ import lombok.Getter;
import lombok.extern.slf4j.Slf4j;
@Slf4j
public final class TradableList<T extends Tradable> implements PersistableEnvelope {
public final class TradableList<T extends Tradable> implements UserThreadMappedPersistableEnvelope {
transient final private Storage<TradableList<T>> storage;
@Getter
private final ObservableList<T> list = FXCollections.observableArrayList();
@ -78,10 +78,10 @@ public final class TradableList<T extends Tradable> implements PersistableEnvelo
.build();
}
public static TradableList fromProto(protobuf.TradableList proto,
CoreProtoResolver coreProtoResolver,
Storage<TradableList<Tradable>> storage,
BtcWalletService btcWalletService) {
public static TradableList<Tradable> fromProto(protobuf.TradableList proto,
CoreProtoResolver coreProtoResolver,
Storage<TradableList<Tradable>> storage,
BtcWalletService btcWalletService) {
List<Tradable> list = proto.getTradableList().stream()
.map(tradable -> {
switch (tradable.getMessageCase()) {

View file

@ -25,7 +25,7 @@ import bisq.core.payment.PaymentAccount;
import bisq.core.proto.CoreProtoResolver;
import bisq.common.proto.ProtoUtil;
import bisq.common.proto.persistable.PersistableEnvelope;
import bisq.common.proto.persistable.UserThreadMappedPersistableEnvelope;
import com.google.protobuf.Message;
@ -49,7 +49,7 @@ import static bisq.core.btc.wallet.Restrictions.getDefaultBuyerSecurityDepositAs
@Slf4j
@Data
@AllArgsConstructor
public final class PreferencesPayload implements PersistableEnvelope {
public final class PreferencesPayload implements UserThreadMappedPersistableEnvelope {
private String userLanguage;
private Country userCountry;
@SuppressWarnings("MismatchedQueryAndUpdateOfCollection")

View file

@ -28,7 +28,7 @@ import bisq.core.support.dispute.mediation.mediator.Mediator;
import bisq.core.support.dispute.refund.refundagent.RefundAgent;
import bisq.common.proto.ProtoUtil;
import bisq.common.proto.persistable.PersistableEnvelope;
import bisq.common.proto.persistable.UserThreadMappedPersistableEnvelope;
import java.util.ArrayList;
import java.util.HashSet;
@ -46,7 +46,7 @@ import javax.annotation.Nullable;
@Slf4j
@Data
@AllArgsConstructor
public class UserPayload implements PersistableEnvelope {
public class UserPayload implements UserThreadMappedPersistableEnvelope {
@Nullable
private String accountId;
@Nullable

View file

@ -141,7 +141,7 @@ public final class Navigation implements PersistedDataHost {
private void queueUpForSave() {
if (currentPath.tip() != null) {
navigationPath.setPath(currentPath.stream().map(Class::getName).collect(Collectors.toList()));
navigationPath.setPath(currentPath.stream().map(Class::getName).collect(Collectors.toUnmodifiableList()));
}
storage.queueUpForSave(navigationPath, 1000);
}

View file

@ -17,7 +17,7 @@
package bisq.network.p2p.peers.peerexchange;
import bisq.common.proto.persistable.PersistableList;
import bisq.common.proto.persistable.PersistableEnvelope;
import com.google.protobuf.Message;
@ -26,19 +26,26 @@ import java.util.List;
import java.util.stream.Collectors;
import lombok.EqualsAndHashCode;
import lombok.Getter;
@EqualsAndHashCode(callSuper = true)
public class PeerList extends PersistableList<Peer> {
@EqualsAndHashCode
public class PeerList implements PersistableEnvelope {
@Getter
private final List<Peer> list;
public PeerList(List<Peer> list) {
super(list);
this.list = list;
}
public int size() {
return list.size();
}
@Override
public Message toProtoMessage() {
return protobuf.PersistableEnvelope.newBuilder()
.setPeerList(protobuf.PeerList.newBuilder()
.addAllPeer(getList().stream().map(Peer::toProtoMessage).collect(Collectors.toList())))
.addAllPeer(list.stream().map(Peer::toProtoMessage).collect(Collectors.toList())))
.build();
}