travitch / persistent-vector

Persistent vectors for Haskell based on array mapped tries
BSD 3-Clause "New" or "Revised" License
27 stars 4 forks source link

No empty #10

Closed treeowl closed 4 years ago

treeowl commented 4 years ago

Remove the empty constructor. This builds on my unsliced branch to (hopefully) get much better performance in common situations. In particular, the RootNode constructor can now be unboxed, so a sequence of modifications to a vector no longer needs to construct all the intermediate root nodes; their components can be stored in registers and on the stack.

treeowl commented 4 years ago

@travitch, I'd very much appreciate your thoughts on this. If you have the time to run some benchmarks (I'm really quite bad at that), I'd appreciate it. I also suspect you may need to do something on your end to enable the GitHub workflows I've plonked into this thing—I really like the way we get multiple test size and test number settings in CI for compact-sequences, and I think that would be valuable here. I've twiddled the Arbitrary instances in preparation for this—previously, we didn't get enough small cases, but now we probably don't get enough big ones without an appropriate -s argument.

treeowl commented 4 years ago

Also also, I feel like this whole PR is supposed to be against something other than master, but I don't have access to create branches in this repo. Maybe it would help if you create an unsliced branch at 2abfeb1656115c27e9c35a945cdc77bee1ae54f0? I don't really know much about git or GitHub, to be totally honest.

treeowl commented 4 years ago

Oh, you enabled the github actions... I just missed that commit. Hmmm... Gonna have to figure that out later.

travitch commented 4 years ago

I think actions aren't running because there are some conflicts. I'll try to clean up these two branches - that should let actions run.

treeowl commented 4 years ago

Superseded by #19.