Skip to content
This repository has been archived by the owner on Dec 22, 2021. It is now read-only.

Add statically known non-empty collections #9

Closed
wants to merge 2 commits into from
Closed

Conversation

julienrf
Copy link
Contributor

I know this work does not follow the design goals but I wanted to give it a try, as an experiment.

This PR removes the head and tail methods from Iterable (they could always be added via an implicit conversion, for backward compatibility, if needed), replaces them with an uncons: Option[(A, C[A])] method, and re-introduces them in a new InhabitedSeq type.

The good thing is that uncons being explicitly partially defined revealed a bug in the current implementation of drop.

The bad thing is that it introduces yet another type in the hierarchy. Also, I didn’t try hard to get the to method preserve “non-emptiness” (e.g. a non empty list transformed into an array should yield a non-empty array) but I think this would require some complex machinery.

@odersky
Copy link
Contributor

odersky commented Dec 28, 2016

Good catch with drop!

Going through an option over a tuple for every head operation is definitely too expensive. So this would make sense only if head and tail were overridden by basically every collection. But then it seems safer to base everything on head and tail from the start, in order not to have an accidental performance drop.

Btw for me the most serious criticism of collections was when people turned back to Java collections because lookup in a Scala mutable map was 5 times as expensive as lookup in a Java map. It turned out the added cost of hashcodes and equality checks in Scala were the main culprit of the slowdown. That cost was introduced in order to have a more reasonable behavior for boxed primitive types. Scala cared about that but Java and C# didn't. The end result was people dropping the Scala abstractions. So, no good deed goes unpunished. Or, better is the enemy of good enough 😄.

@julienrf julienrf changed the base branch from mutable-immutable to master December 29, 2016 09:57
@julienrf julienrf mentioned this pull request Jan 3, 2017
@edmundnoble
Copy link

Manipulating collections through their methods will most of the time preclude using uncons, assuming those methods are overridden. If it made sense for them to use uncons, we could provide an unsafeHead and unsafeTail operation which would be protected. Practically, if you care about performance, you already have the same problem in the current collections hierarchy if you forget to overload something: just look at my PR overloading Queue.++ for a recent example. It's orders of magnitude slower just because it wasn't overloaded. If we want to close this accidental performance loss gap we will likely be helped more by benchmarks that work over generic interface traits like Seq and Iterable and can be utilized by implementors of new collections to make sure they haven't forgotten to overload something in the interface and dropped off a performance cliff.

@julienrf
Copy link
Contributor Author

julienrf commented Jan 4, 2017

I just added a commit that introduces protected methods unsafeHead and unsafeTail to remove the Option and tuple allocation in some places.

@julienrf
Copy link
Contributor Author

I think this one will introduce too much complexity :(

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants