Add strict vector #488

Shimuuar · 2024-03-18T17:56:40Z

Implementation follows discussion in issues above and virtually identical to one in strict-containers. Only difference fromArray and unsafeFromArraySlice now force all elements of returned vector to WHNF. It changes complexity to O(n) so to compensate lazyFromArray and unsafeLazyFromArraySlice are added, they retain O(1) complexity but user has to deal with strictness.

This is draft since there's still some work to do with documentation.

We need to decide to prefix to use in examples. V. is used by lazy vector, VS by storable. One option
I think it would be better to switch examples in generic vector to strict vector. It's better default data structure
It's worth making a new release soon after this PR is merged, so I think I should just slap @since 0.13.2.0 on each function in Strict modules

Also

Missing LICENSE file for vector-bench-papi
Small cleanup in test suite

This is essentially a copy of lazy boxed vector with - MVector.basicUnsafeWrite - MVector.basicUnsafeReplicate - Vector.elemseq are made strict Tests are copy of lazy vector's tests as well

fromArray reduces all elements of array to WHNF but that changes complexity from O(1) to O(n). To compensate we provide lazy variant which pass all responsibility to user

Strict vectors are generally better default

Shimuuar · 2024-03-22T18:35:05Z

At this point PR is pretty much ready

Bodigrim · 2024-03-22T20:04:21Z

@infinity0 could you possibly review?

Shimuuar · 2024-03-22T22:11:56Z

Basically it's same as version from strict-containers. Only differences are strict fromArray & fromMutableArray, added lazyFromArray and haddocks.

I also switched examples in D.V.Generic to strict vectors but that's purely documentation change

Bodigrim

Awesome!

vector/src/Data/Vector/Strict/Mutable.hs

Bodigrim · 2024-03-23T20:31:56Z

vector/src/Data/Vector/Strict.hs

+
+
+-- | Boxed vectors, supporting efficient slicing.
+data Vector a = Vector {-# UNPACK #-} !Int


Could it be a newtype over lazy boxed vectors? I imagine it would save us some instance definitions.

That should be possible. Probably most importantly for cheap conversion between strict and lazy vectors.

But we should be careful with instances. Anything that produces vectors should be implemented in terms of operations over strict vectors.

I think it's worth to employ newtypes if possible, to demonstrate clearly that it is the same data structure just with different instances.

Bodigrim · 2024-03-23T20:33:15Z

vector/src/Data/Vector/Strict/Mutable.hs

+type role MVector nominal representational
+
+-- | Mutable boxed vectors keyed on the monad they live in ('IO' or @'ST' s@).
+data MVector s a = MVector { _offset :: {-# UNPACK #-} !Int


Same question: could this be a newtype over lazy boxed MVector?

Co-authored-by: ˌbodʲɪˈɡrʲim <andrew.lelechenko@gmail.com>

@Bodigrim

This is done over @Bodigrim's suggestion. This allowed to drop quite bit of code

However we can't derive Eq & Ord since DerivingStrategies are only available since 8.2

It seems cabal decided on plan with deepseq<1.4.3

Shimuuar · 2024-03-24T14:53:18Z

@Bodigrim switching to a newtype worked out quite well and allowed to drop a lot of duplicated code. It doesn't work all that well for instances since DerivingStrategies are only available since 8.2. So it's not possible to instruct GHC to derive say Ord.

I also added function for conversion between strict and lazy vectors.

infinity0 · 2024-03-26T13:45:35Z

vector/src/Data/Vector/Strict.hs

+-- of holding any Haskell value). It is possible to create vector
+-- which contain bottom elements, either by using mutable interfaces
+-- (see "Data.Vector.Strict.Mutable") or functions that don't preserve
+-- strictness ('lazyFromArray').


What's the use case for this? Just seems like a potential source of bugs and/or making existing bugs harder to find. If somebody sees this strict type they are going to assume it doesn't contain bottom elements, but this design seems to suggest otherwise.

IMO a type that is advertised as "strict" should have no possible external (user) code path that can lead it to containing non-WHNF elements, even if it's internally implemented using lazy structures.

they retain O(1) complexity but user has to deal with strictness.

I don't think this is a good use-case, if the user selects a strict data type they should be aware that there will be potential performance penalties for certain operations, we shouldn't expose a non-strict operation for them to "regain performance", they should instead just pick the lazy type.

edit: If a strict struct has to be passed around a bunch of different libraries, any one of them could make a mistake making this trade-off decision, and it will be a massive pain to track down the culprit.

On one hand it's one of many unsafe functions where all responsibility on maintaining invariants is passed to the caller. So it seems that it would be in a good company.

However here invariant is "all elements are in WHNF" And it's known to be rather tricky to maintain. And even worse it's difficult to check and invariant violations could be invisible when all test look fine and then it fail on prod due to space leak.

So I think it would be better to drop these functions and if someone really need these it could be discussed.

These functions require user to maintain invariant that all element of vector is in WHNF. And it's well known that this insvariant is difficult to enforce and its violations are usually invisible. Such spec leaks may be invisibe during testing only to cause pproblem in production. So it's better to remove them unless good use case for them appears

Shimuuar · 2024-03-30T17:15:48Z

I removed function that don't enforce strictness: lazyFromLazy, etc.

infinity0 · 2024-03-31T12:14:00Z

vector/src/Data/Vector/Strict.hs

@@ -23,11 +23,9 @@
 -- Immutable strict boxed vectors (that is, polymorphic arrays capable
 -- of holding any Haskell value). It is possible to create vector
 -- which contain bottom elements, either by using mutable interfaces


Can drop "either" here now, also might be good to clarify the vectors that might contain bottom elements are the original data type, not this strict type.

I reworded module comment in order to make it more clear

Shimuuar · 2024-04-08T17:37:11Z

I'm going to merge this PR on next weekend if there are no objections

lehins

I skimmed through the PR. Looks like it is just more of the same as regular lazy vector. I looked at all the entry points where lazy values could enter into the strict vector and it looks like they are all covered.

So, no objections from me.

Shimuuar force-pushed the strict-vector branch from 344fe6a to c911374 Compare March 18, 2024 18:15

Shimuuar added 11 commits March 22, 2024 20:31

Missing LICENSE file for vector-bench-papi

9f382f0

We don't need Setup.hs anymore

8e61180

Inline module Test.Vector into Main

c42e544

base provide stripPrefix since at least 4.9

1e7efb5

Implement strict boxed vectors

06b6cf6

This is essentially a copy of lazy boxed vector with - MVector.basicUnsafeWrite - MVector.basicUnsafeReplicate - Vector.elemseq are made strict Tests are copy of lazy vector's tests as well

Correctly implement fromArray for strict vector

f672ad6

fromArray reduces all elements of array to WHNF but that changes complexity from O(1) to O(n). To compensate we provide lazy variant which pass all responsibility to user

Add strict vectors to doctests

0d45300

Add @SInCE annotations for all functions in Strict modules

78db774

fromMutableArray now evaluates all array eleemnts to WHNF

7aea84f

Better documentation for strict vector

b7a80ba

Switch examples in D.V.Generic to strict vectors

e91e30a

Strict vectors are generally better default

Shimuuar force-pushed the strict-vector branch from c911374 to e91e30a Compare March 22, 2024 18:33

Shimuuar marked this pull request as ready for review March 22, 2024 18:34

Bodigrim reviewed Mar 23, 2024

View reviewed changes

Shimuuar and others added 7 commits March 24, 2024 00:31

Fix haddock

48e6ef7

Co-authored-by: ˌbodʲɪˈɡrʲim <andrew.lelechenko@gmail.com>

Use correct constructor name

c623db7

Represent strict boxed vector as newtype over lazy one

3530f8f

This is done over @Bodigrim's suggestion. This allowed to drop quite bit of code

Derive some of the instances

3dcf821

However we can't derive Eq & Ord since DerivingStrategies are only available since 8.2

Implement conversion between strict and lazy vectors

36b12ef

Fix doctests

fe1ddef

Fix build on GHC8.0

8f60f2a

It seems cabal decided on plan with deepseq<1.4.3

infinity0 reviewed Mar 26, 2024

View reviewed changes

infinity0 reviewed Mar 31, 2024

View reviewed changes

Reword module comment for immutable strict vectors

96b0cc0

lehins approved these changes Apr 8, 2024

View reviewed changes

Shimuuar merged commit 219b33f into haskell:master Apr 14, 2024
22 checks passed

Shimuuar deleted the strict-vector branch April 14, 2024 16:21

Shimuuar mentioned this pull request Apr 14, 2024

Implementing a value-strict Vector #380

Closed

lehins mentioned this pull request Sep 20, 2024

Need a Boxed instance for Unboxed (to be able to unbox user data types with some boxed fields) #503

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add strict vector #488

Add strict vector #488

Shimuuar commented Mar 18, 2024

Shimuuar commented Mar 22, 2024

Bodigrim commented Mar 22, 2024

Shimuuar commented Mar 22, 2024

Bodigrim left a comment

Bodigrim Mar 23, 2024

Shimuuar Mar 23, 2024

Bodigrim Mar 23, 2024

Bodigrim Mar 23, 2024

Shimuuar commented Mar 24, 2024

infinity0 Mar 26, 2024

infinity0 Mar 26, 2024 •

edited

Loading

Shimuuar Mar 27, 2024

Shimuuar commented Mar 30, 2024

infinity0 Mar 31, 2024

Shimuuar Mar 31, 2024

Shimuuar commented Apr 8, 2024

lehins left a comment



		-- \| Boxed vectors, supporting efficient slicing.
		data Vector a = Vector {-# UNPACK #-} !Int

Add strict vector #488

Add strict vector #488

Conversation

Shimuuar commented Mar 18, 2024

Shimuuar commented Mar 22, 2024

Bodigrim commented Mar 22, 2024

Shimuuar commented Mar 22, 2024

Bodigrim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shimuuar commented Mar 24, 2024

Choose a reason for hiding this comment

infinity0 Mar 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shimuuar commented Mar 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shimuuar commented Apr 8, 2024

lehins left a comment

Choose a reason for hiding this comment

infinity0 Mar 26, 2024 •

edited

Loading