ClojureScript

repeated assoc into map eventually drops meta data

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Environment:
    [org.clojure/clojurescript "0.0-1552"]

Description

When running this loop, then the map's meta is lost after 32 assocs:

(loop [i 0, m (with-meta {} {:foo :bar})]
  (when (<= i 34)
    (.log js/console i (pr-str (meta m)))
    (recur (inc i) (assoc m (str i) i))))

The last 4 lines of output read

31 {:foo :bar}
  32 {:foo :bar}
  33 nil
  34 nil

cljs.core.ObjMap/HASHMAP_THRESHOLD happens to be 32, so I guess
there is a connection.

Activity

Michał Marczyk made changes -
Field Original Value New Value
Assignee Michał Marczyk [ michalmarczyk ]
Michał Marczyk made changes -
Attachment 0001-CLJS-461-preserve-metadata-on-automatic-ObjMap-to-PH.patch [ 11815 ]
Hide
Michał Marczyk added a comment -

obj-map->hash-map failed to take into account the fact that transients don't support metadata. The attached patch fixes this.

Show
Michał Marczyk added a comment - obj-map->hash-map failed to take into account the fact that transients don't support metadata. The attached patch fixes this.
Hide
Michał Marczyk added a comment -

Just realized that this also happens with PAMs. New patch addresses the issue in both places. (Also tweaks the PAM->PHM conversion – no more transient creation for a single assoc.)

Show
Michał Marczyk added a comment - Just realized that this also happens with PAMs. New patch addresses the issue in both places. (Also tweaks the PAM->PHM conversion – no more transient creation for a single assoc.)
Michał Marczyk made changes -
Attachment 0001-CLJS-461-preserve-metadata-on-automatic-map-conversi.patch [ 11816 ]
Michał Marczyk made changes -
Attachment 0001-CLJS-461-preserve-metadata-on-automatic-ObjMap-to-PH.patch [ 11815 ]
David Nolen made changes -
Description When running this loop, then the map's meta is lost after 32 assocs:

  (loop [i 0, m (with-meta {} {:foo :bar})]
    (when (<= i 34)
      (.log js/console i (pr-str (meta m)))
      (recur (inc i) (assoc m (str i) i))))

The last 4 lines of output read

  31 {:foo :bar}
  32 {:foo :bar}
  33 nil
  34 nil

cljs.core.ObjMap/HASHMAP_THRESHOLD happens to be 32, so I guess
there is a connection.
When running this loop, then the map's meta is lost after 32 assocs:

{code}
(loop [i 0, m (with-meta {} {:foo :bar})]
  (when (<= i 34)
    (.log js/console i (pr-str (meta m)))
    (recur (inc i) (assoc m (str i) i))))
{code}

The last 4 lines of output read

{code}
  31 {:foo :bar}
  32 {:foo :bar}
  33 nil
  34 nil
{code}

cljs.core.ObjMap/HASHMAP_THRESHOLD happens to be 32, so I guess
there is a connection.
Hide
David Nolen added a comment -

I tried the patch and it did not seem to resolve the case above. Could we get a patch that includes a test case? Thanks!

Show
David Nolen added a comment - I tried the patch and it did not seem to resolve the case above. Could we get a patch that includes a test case? Thanks!
Hide
Michał Marczyk added a comment -

I tried it again at a Rhino REPL and for me it resolves both problems (OM and PAM). New patch with test cases based on the form from the ticket attached. (The tests do fail on master.)

Show
Michał Marczyk added a comment - I tried it again at a Rhino REPL and for me it resolves both problems (OM and PAM). New patch with test cases based on the form from the ticket attached. (The tests do fail on master.)
Michał Marczyk made changes -
Attachment 0005-CLJS-461-preserve-metadata-on-automatic-map-conversi.patch [ 11819 ]
Michał Marczyk made changes -
Attachment 0005-CLJS-461-preserve-metadata-on-automatic-map-conversi.patch [ 11819 ]
Michał Marczyk made changes -
Attachment 0001-CLJS-461-preserve-metadata-on-automatic-map-conversi.patch [ 11816 ]
Michał Marczyk made changes -
Attachment 0001-CLJS-461-preserve-metadata-on-automatic-map-conversi.patch [ 11820 ]
Michał Marczyk made changes -
Attachment 0001-CLJS-461-preserve-metadata-on-automatic-map-conversi.patch [ 11820 ]
Hide
Michał Marczyk added a comment -

Tweaked patch with ticket number mentioned in a comment above the new test cases.

Show
Michał Marczyk added a comment - Tweaked patch with ticket number mentioned in a comment above the new test cases.
Michał Marczyk made changes -
David Nolen made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Resolved [ 5 ]
David Nolen made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: