ClojureScript

Set literal and hash-set cleanup

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Declined
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Environment:
    Clojurescript 0.0-2127

Description

Some sets drop values, causing loss of data.

The smallest example I've been able to find is:

#{[1 4] [2 4] [3 4] [0 3] [1 3] [2 3] [3 3] [3 2] [4 2]}
Drops: [1 4] [3 4] [1 3] [3 3]

hash-set also drops values. Though they are different.

(hash-set [1 4] [2 4] [3 4] [0 3] [1 3] [2 3] [3 3] [3 2] [4 2])
Drops: [2 4] [0 3] [2 3] [3 2]

Re-ordering the values appears to make no difference.

I have found no instance where sorted-set drops values.

Activity

Hide
Francis Avila added a comment -

This is a bug in PersistentHashSet.fromArray that causes every other item to be skipped when the input is longer than the PersistentArrayMap.HASHMAP_THRESHOLD (which is 8). Demonstration:

ClojureScript:cljs.user> (hash-set [0] [1] [2] [3] [4] [5] [6] [7] [8])
#{[2] [4] [6] [8] [0]}
ClojureScript:cljs.user> (hash-set [0] [1] [2] [3] [4] [5] [6] [7])   
#{[1] [2] [3] [4] [5] [6] [7] [0]}

Working on a patch and test.

Show
Francis Avila added a comment - This is a bug in PersistentHashSet.fromArray that causes every other item to be skipped when the input is longer than the PersistentArrayMap.HASHMAP_THRESHOLD (which is 8). Demonstration:
ClojureScript:cljs.user> (hash-set [0] [1] [2] [3] [4] [5] [6] [7] [8])
#{[2] [4] [6] [8] [0]}
ClojureScript:cljs.user> (hash-set [0] [1] [2] [3] [4] [5] [6] [7])   
#{[1] [2] [3] [4] [5] [6] [7] [0]}
Working on a patch and test.
Hide
Francis Avila added a comment -

Nevermind, looks like David fixed this already.

Show
Francis Avila added a comment - Nevermind, looks like David fixed this already.
Hide
Francis Avila added a comment -

Some dead code was left in PersistentHashSet.fromArray (no-clone arg and arr let-binding). Attached a patch that cleans this up.

Show
Francis Avila added a comment - Some dead code was left in PersistentHashSet.fromArray (no-clone arg and arr let-binding). Attached a patch that cleans this up.
Hide
David Nolen added a comment -

Thanks for the patch but remember we need CAs to apply them.

The current hash-set construction implementation misses an optimization opportunity. We could build hash-sets much faster if we know that we have only unique constants at compile time.

Show
David Nolen added a comment - Thanks for the patch but remember we need CAs to apply them. The current hash-set construction implementation misses an optimization opportunity. We could build hash-sets much faster if we know that we have only unique constants at compile time.
Hide
David Nolen added a comment -

Need to implement the hash-set construction optimization, the old removed approach is faster when we know we have a set literal containing only constants.

Show
David Nolen added a comment - Need to implement the hash-set construction optimization, the old removed approach is faster when we know we have a set literal containing only constants.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: