Approach: Create a TransientSubVector based on an underlying TransientVector.

Two assumptions:

It's okay for TransientSubVector to delegate the ensureEditable functionality to the underlying TransientVector (sometimes explicitly, sometimes implicitly) - calling ensureEditable explicitly also requires that the field for the underlying vector be the concrete TransientVector type rather than the ITransientVector interface.

When an operation that would throw an exception on a PersistentVector happens from the wrong thread (or after persistent!), we throw that exception rather than the IllegalAccessError that transients throw when accessed inappropriately.

Attachments

Activity

Confirmed. APersistentVector$SubVector does not implement IEditableCollection.

The current implementation of TransientVector depends on implementation details of PersistentVector, so it is not a trivial fix. The simplest fix might be to implement IEditableCollection.asTransient in SubVector by creating a new PersistentVector, but I do not know the performance implications.

Stuart Sierra
added a comment - 31/May/11 9:28 AM Confirmed. APersistentVector$SubVector does not implement IEditableCollection.
The current implementation of TransientVector depends on implementation details of PersistentVector, so it is not a trivial fix. The simplest fix might be to implement IEditableCollection.asTransient in SubVector by creating a new PersistentVector, but I do not know the performance implications.

Gary Fredericks
added a comment - 25/May/13 8:11 PM We could get the same performance characteristics as SubVector by creating a TransientSubVector based on an underlying TransientVector, right?
Preparing a patch to that effect.

It's okay for TransientSubVector to delegate the ensureEditable
functionality to the underlying TransientVector (sometimes
explicitely, sometimes implicitely) – calling ensureEditable
explicitely also requires that the field for the underlying vector
be the concrete TransientVector type rather than the
ITransientVector interface.

When an operation that would throw an exception on a
PersistentVector happens from the wrong thread (or after
persistent!), we throw that exception rather than the
IllegalAccessError that transients throw when accessed
inappropriately.

It's okay for TransientSubVector to delegate the ensureEditable
functionality to the underlying TransientVector (sometimes
explicitely, sometimes implicitely) – calling ensureEditable
explicitely also requires that the field for the underlying vector
be the concrete TransientVector type rather than the
ITransientVector interface.

When an operation that would throw an exception on a
PersistentVector happens from the wrong thread (or after
persistent!), we throw that exception rather than the
IllegalAccessError that transients throw when accessed
inappropriately.

PersistentVector.SubVector expects to work on anything that implements IPersistentVector. Note that this includes concrete types such as MapEntry and LazilyPersistentVector, but could also be any user-implemented type IPersistentVector type too. TransientSubVector is making the assumption that the IPersistentVector in a SubVector question is also an IEditableCollection (that can be converted to be transient). Note that while PersistentVector implements TransientVector (and IEditableCollection), APersistentVector does not. To really implement this in tandem with SubVector, I think you would need to guarantee that IPersistentVector extended IEditableCollection and I don't think that's something we want to do.

I don't see an easy solution. Any time I see all these modifiers (Transient, Sub, etc) being created in different combinations, it is a clear sign that independent kinds of functionality are being remixed into single inheritance OO trees. You can see the same thing in most collection libraries (even Java's - need a ConcurrentIdentitySortedMap? too bad!).

Alex Miller
added a comment - 11/Oct/13 4:17 PM I think there are some assumptions being made in this patch about the class structure here that do not hold. The structure is, admittedly, quite twisty.
A counter-example that highlights one of a few subtypes of APersistentVector that are not PersistentVector (like MapEntry):

PersistentVector.SubVector expects to work on anything that implements IPersistentVector. Note that this includes concrete types such as MapEntry and LazilyPersistentVector, but could also be any user-implemented type IPersistentVector type too. TransientSubVector is making the assumption that the IPersistentVector in a SubVector question is also an IEditableCollection (that can be converted to be transient). Note that while PersistentVector implements TransientVector (and IEditableCollection), APersistentVector does not. To really implement this in tandem with SubVector, I think you would need to guarantee that IPersistentVector extended IEditableCollection and I don't think that's something we want to do.
I don't see an easy solution. Any time I see all these modifiers (Transient, Sub, etc) being created in different combinations, it is a clear sign that independent kinds of functionality are being remixed into single inheritance OO trees. You can see the same thing in most collection libraries (even Java's - need a ConcurrentIdentitySortedMap? too bad!).
Needs more thought.

Patch CLJ-787-p1.patch no longer applies cleanly to latest master, but it is only because of some new tests added to the transients.clj file since the patch was created, so it is trivial to update in that sense. Not updating it now due to other more significant issues with the patch described above.

Andy Fingerhut
added a comment - 08/Nov/13 10:17 AM Patch CLJ-787-p1.patch no longer applies cleanly to latest master, but it is only because of some new tests added to the transients.clj file since the patch was created, so it is trivial to update in that sense. Not updating it now due to other more significant issues with the patch described above.