Skip to content

Commit

Permalink
CLJ-1224: cache hasheq and hashCode for records
Browse files Browse the repository at this point in the history 
Signed-off-by: Stuart Halloway <stu@cognitect.com>
  • Loading branch information
@Bronsa @stuarthalloway
Bronsa authored and stuarthalloway committed Sep 7, 2016
1 parent 1318655 commit a1c3daf
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 22 deletions.
43 changes: 29 additions & 14 deletions src/clj/clojure/core_deftype.clj
View file
Expand Up @@ -156,7 +156,9 @@
hinted-fields fields
fields (vec (map #(with-meta % nil) fields))
base-fields fields
fields (conj fields '__meta '__extmap)
fields (conj fields '__meta '__extmap
'^:unsynchronized-mutable __hash
'^:unsynchronized-mutable __hasheq)
type-hash (hash classname)]
(when (some #{:volatile-mutable :unsynchronized-mutable} (mapcat (comp keys meta) hinted-fields))
(throw (IllegalArgumentException. ":volatile-mutable or :unsynchronized-mutable not supported for record fields")))
Expand All @@ -168,8 +170,18 @@
(eqhash [[i m]]
[(conj i 'clojure.lang.IHashEq)
(conj m
`(hasheq [this#] (bit-xor ~type-hash (clojure.lang.APersistentMap/mapHasheq this#)))
`(hashCode [this#] (clojure.lang.APersistentMap/mapHash this#))
`(hasheq [this#] (let [hq# ~'__hasheq]
(if (zero? hq#)
(let [h# (int (bit-xor ~type-hash (clojure.lang.APersistentMap/mapHasheq this#)))]
(set! ~'__hasheq h#)
h#)
hq#)))
`(hashCode [this#] (let [hash# ~'__hash]
(if (zero? hash#)
(let [h# (clojure.lang.APersistentMap/mapHash this#)]
(set! ~'__hash h#)
h#)
hash#)))
`(equals [this# ~gs] (clojure.lang.APersistentMap/mapEquals this# ~gs)))])
(iobj [[i m]]
[(conj i 'clojure.lang.IObj)
Expand Down Expand Up @@ -220,12 +232,12 @@
`(assoc [this# k# ~gs]
(condp identical? k#
~@(mapcat (fn [fld]
[(keyword fld) (list* `new tagname (replace {fld gs} fields))])
[(keyword fld) (list* `new tagname (replace {fld gs} (remove '#{__hash __hasheq} fields)))])
base-fields)
(new ~tagname ~@(remove #{'__extmap} fields) (assoc ~'__extmap k# ~gs))))
(new ~tagname ~@(remove '#{__extmap __hash __hasheq} fields) (assoc ~'__extmap k# ~gs))))
`(without [this# k#] (if (contains? #{~@(map keyword base-fields)} k#)
(dissoc (with-meta (into {} this#) ~'__meta) k#)
(new ~tagname ~@(remove #{'__extmap} fields)
(new ~tagname ~@(remove '#{__extmap __hash __hasheq} fields)
(not-empty (dissoc ~'__extmap k#))))))])
(ijavamap [[i m]]
[(conj i 'java.util.Map 'java.io.Serializable)
Expand All @@ -243,8 +255,11 @@
`(entrySet [this#] (set this#)))])
]
(let [[i m] (-> [interfaces methods] irecord eqhash iobj ilookup imap ijavamap)]
`(deftype* ~(symbol (name (ns-name *ns*)) (name tagname)) ~classname ~(conj hinted-fields '__meta '__extmap)
:implements ~(vec i)
`(deftype* ~(symbol (name (ns-name *ns*)) (name tagname)) ~classname
~(conj hinted-fields '__meta '__extmap
'^int ^:unsynchronized-mutable __hash
'^int ^:unsynchronized-mutable __hasheq)
:implements ~(vec i)
~@(mapcat identity opts)
~@m))))))

Expand Down Expand Up @@ -280,7 +295,7 @@
[fields name]
(when-not (vector? fields)
(throw (AssertionError. "No fields vector given.")))
(let [specials #{'__meta '__extmap}]
(let [specials '#{__meta __hash __hasheq __extmap}]
(when (some specials fields)
(throw (AssertionError. (str "The names in " specials " cannot be used as field names for types or records.")))))
(let [non-syms (remove symbol? fields)]
Expand Down Expand Up @@ -357,9 +372,9 @@
Two constructors will be defined, one taking the designated fields
followed by a metadata map (nil for none) and an extension field
map (nil for none), and one taking only the fields (using nil for
meta and extension fields). Note that the field names __meta
and __extmap are currently reserved and should not be used when
defining your own records.
meta and extension fields). Note that the field names __meta,
__extmap, __hash and __hasheq are currently reserved and should not
be used when defining your own records.
Given (defrecord TypeName ...), two factory functions will be
defined: ->TypeName, taking positional parameters for the fields,
Expand Down Expand Up @@ -465,8 +480,8 @@
writes the .class file to the *compile-path* directory.
One constructor will be defined, taking the designated fields. Note
that the field names __meta and __extmap are currently reserved and
should not be used when defining your own types.
that the field names __meta, __extmap, __hash and __hasheq are currently
reserved and should not be used when defining your own types.
Given (deftype TypeName ...), a factory function called ->TypeName
will be defined, taking positional parameters for the fields"
Expand Down
73 changes: 66 additions & 7 deletions src/jvm/clojure/lang/Compiler.java
View file
Expand Up @@ -4414,8 +4414,34 @@ void compile(String superName, String[] interfaceNames, boolean oneTimeUse) thro
ctorgen.visitCode();
ctorgen.loadThis();
ctorgen.loadArgs();
for(int i=0;i<altCtorDrops;i++)
ctorgen.visitInsn(Opcodes.ACONST_NULL);

ctorgen.visitInsn(Opcodes.ACONST_NULL); //__meta
ctorgen.visitInsn(Opcodes.ACONST_NULL); //__extmap
ctorgen.visitInsn(Opcodes.ICONST_0); //__hash
ctorgen.visitInsn(Opcodes.ICONST_0); //__hasheq

ctorgen.invokeConstructor(objtype, new Method("<init>", Type.VOID_TYPE, ctorTypes));

ctorgen.returnValue();
ctorgen.endMethod();

// alt ctor no __hash, __hasheq
altCtorTypes = new Type[ctorTypes.length-2];
for(int i=0;i<altCtorTypes.length;i++)
altCtorTypes[i] = ctorTypes[i];

alt = new Method("<init>", Type.VOID_TYPE, altCtorTypes);
ctorgen = new GeneratorAdapter(ACC_PUBLIC,
alt,
null,
null,
cv);
ctorgen.visitCode();
ctorgen.loadThis();
ctorgen.loadArgs();

ctorgen.visitInsn(Opcodes.ICONST_0); //__hash
ctorgen.visitInsn(Opcodes.ICONST_0); //__hasheq

ctorgen.invokeConstructor(objtype, new Method("<init>", Type.VOID_TYPE, ctorTypes));

Expand Down Expand Up @@ -7766,7 +7792,11 @@ static ObjExpr build(IPersistentVector interfaceSyms, IPersistentVector fieldSym
//use array map to preserve ctor order
ret.closes = new PersistentArrayMap(closesvec);
ret.fields = fmap;
for(int i=fieldSyms.count()-1;i >= 0 && (((Symbol)fieldSyms.nth(i)).name.equals("__meta") || ((Symbol)fieldSyms.nth(i)).name.equals("__extmap"));--i)
for(int i=fieldSyms.count()-1;i >= 0 && (((Symbol)fieldSyms.nth(i)).name.equals("__meta")
|| ((Symbol)fieldSyms.nth(i)).name.equals("__extmap")
|| ((Symbol)fieldSyms.nth(i)).name.equals("__hash")
|| ((Symbol)fieldSyms.nth(i)).name.equals("__hasheq")
);--i)
ret.altCtorDrops++;
}
//todo - set up volatiles
Expand Down Expand Up @@ -7910,8 +7940,35 @@ static Class compileStub(String superName, NewInstanceExpr ret, String[] interfa
ctorgen.visitCode();
ctorgen.loadThis();
ctorgen.loadArgs();
for(int i=0;i<ret.altCtorDrops;i++)
ctorgen.visitInsn(Opcodes.ACONST_NULL);

ctorgen.visitInsn(Opcodes.ACONST_NULL); //__meta
ctorgen.visitInsn(Opcodes.ACONST_NULL); //__extmap
ctorgen.visitInsn(Opcodes.ICONST_0); //__hash
ctorgen.visitInsn(Opcodes.ICONST_0); //__hasheq

ctorgen.invokeConstructor(Type.getObjectType(COMPILE_STUB_PREFIX + "/" + ret.internalName),
new Method("<init>", Type.VOID_TYPE, ctorTypes));

ctorgen.returnValue();
ctorgen.endMethod();

// alt ctor no __hash, __hasheq
altCtorTypes = new Type[ctorTypes.length-2];
for(int i=0;i<altCtorTypes.length;i++)
altCtorTypes[i] = ctorTypes[i];

alt = new Method("<init>", Type.VOID_TYPE, altCtorTypes);
ctorgen = new GeneratorAdapter(ACC_PUBLIC,
alt,
null,
null,
cv);
ctorgen.visitCode();
ctorgen.loadThis();
ctorgen.loadArgs();

ctorgen.visitInsn(Opcodes.ICONST_0); //__hash
ctorgen.visitInsn(Opcodes.ICONST_0); //__hasheq

ctorgen.invokeConstructor(Type.getObjectType(COMPILE_STUB_PREFIX + "/" + ret.internalName),
new Method("<init>", Type.VOID_TYPE, ctorTypes));
Expand Down Expand Up @@ -8006,9 +8063,11 @@ protected void emitStatics(ClassVisitor cv) {
}
}

mv.visitInsn(ACONST_NULL);
mv.visitVarInsn(ALOAD, 0);
mv.visitInsn(ACONST_NULL); //__meta
mv.visitVarInsn(ALOAD, 0); //__extmap
mv.visitMethodInsn(INVOKESTATIC, "clojure/lang/RT", "seqOrElse", "(Ljava/lang/Object;)Ljava/lang/Object;");
mv.visitInsn(ICONST_0); //__hash
mv.visitInsn(ICONST_0); //__hasheq
mv.visitMethodInsn(INVOKESPECIAL, className, "<init>", ctor.getDescriptor());
mv.visitInsn(ARETURN);
mv.visitMaxs(4+fieldCount, 1+fieldCount);
Expand Down
15 changes: 14 additions & 1 deletion test/clojure/test_clojure/data_structures.clj
View file
Expand Up @@ -1287,4 +1287,17 @@
(defspec seq-and-iter-match-for-structs
identity
[^{:tag clojure.test-clojure.data-structures/gen-struct} s]
(seq-iter-match s s))
(seq-iter-match s s))

(deftest record-hashing
(let [r (->Rec 1 1)
_ (hash r)
r2 (assoc r :c 2)]
(is (= (hash (->Rec 1 1)) (hash r)))
(is (= (hash r) (hash (with-meta r {:foo 2}))))
(is (not= (hash (->Rec 1 1)) (hash (assoc (->Rec 1 1) :a 2))))
(is (not= (hash (->Rec 1 1)) (hash r2)))
(is (not= (hash (->Rec 1 1)) (hash (assoc r :a 2))))
(is (= (hash (->Rec 1 1)) (hash (assoc r :a 1))))
(is (= (hash (->Rec 1 1)) (hash (dissoc r2 :c))))
(is (= (hash (->Rec 1 1)) (hash (dissoc (assoc r :c 1) :c))))))

0 comments on commit a1c3daf

Please sign in to comment.