clojure-dev

Issues: https://clojure.atlassian.net/browse/CLJ | Guide: https://insideclojure.org/2015/05/01/contributing-clojure/
borkdude 2019-08-01T08:39:35.113800Z

let me know if that's somehow useful

2019-08-01T17:30:28.114800Z

If anyone has both (a) good knowledge of how the edit fields are used in Clojure's transient data structure implementations and (b) time to kill answering questions about them, I have a few.

2019-08-01T17:35:52.116600Z

In particular, probably the most important question I have is: (a) Should it be an invariant that all edit fields within the same tree all point at the same Java object (if that should be an invariant, then there is a bug in Clojure's implementation where it doesn't preserve that condition). (b) If that shouldn't be an invariant, then how do they enforce what they are intended to enforce?

alexmiller 2019-08-01T18:03:06.117100Z

I don't have code up atm but I believe that is the field that used to be an invariant, but no longer is

alexmiller 2019-08-01T18:03:43.117400Z

assuming that's the thread tracking field

alexmiller 2019-08-01T18:04:03.118100Z

originally transient data structures required that all changes to a transient happened in the same thread

2019-08-01T18:04:38.119200Z

It is the thread-tracking field. I know that it used to be enforced "more strongly" in the past, and now much less so.

alexmiller 2019-08-01T18:04:41.119300Z

in clojure 1.6 we relaxed this to "no more than one thread at a time" so that transients can be modified by go blocks which may get assigned to different threads over time from the go block pool

2019-08-01T18:05:51.119900Z

I think I am finding the answers to my original question, a la rubber duck (and probably almost having the answer before asking).

alexmiller 2019-08-01T18:05:59.120100Z

iirc we removed some of the checks but not all of the tracking

2019-08-01T18:06:58.121200Z

The edit nodes are still necessary, I think, to know which tree nodes are "owned" by this transient (and it made clones of them so it is safe for it to mutate them), vs. ones that might still be shared with immutable data structures.

alexmiller 2019-08-01T18:10:05.122700Z

that sounds right. it's been a few years. :)

2019-08-01T22:27:25.126600Z

Hmmm. And I think I may have finally answered a question that has long bugged me: Is it safe to pass transient collections from one thread to another, and if so, why? Most collections have a bunch of Java arrays in their implementation, which by themselves are not always published safely to other threads in all cases, only some. For persistent collections, I believe all Java arrays are fully written during constructor calls, and then a reference to those arrays are stored in final fields of the persistent collection implementation, and final fields have special rules in the JMM for being safe to publish to other threads.

đź‘Ź 1
2019-08-01T22:29:35.128500Z

I believe the answer for transients has always been that they are safe because all of the transient Java objects have volatile fields, so as long as every operation on a transient reads or writes at least one of those fields after modifying an array, and the next thread reads one of them (which ensureEditable() does in all of the published methods), then the next thread should read everything up to date, too.

2019-08-01T22:31:56.128900Z

Wow, it would be so easy to break that with otherwise innocent-looking changes to some of the transient methods.

2019-08-01T22:46:07.130600Z

OK, that hasn't always been the answer, because Alex made those transient object fields volatile at the same time that the same-thread-only-can-update restriction was removed for transients. Still digging on this for a bit more, and may write up some notes in case anyone wants to read and/or double-check them.

đź‘Ť 1
alexmiller 2019-08-01T23:10:40.131700Z

When they were enforced to be single thread only it wouldn’t matter

2019-08-01T23:30:14.132500Z

It still needed a way to safely publish updates made while transient, in the implementation of persistent!, which I believe were in place since transients were introduced.

2019-08-01T23:41:14.133200Z

Hmmm. Looks like core.rrb-vector does not using volatiles for transients, the way the core vector type does. Something to improve on there.

alexmiller 2019-08-01T23:49:52.133500Z

Yes, I would think so