Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify FreeC and Algebra types. #1610

Merged
merged 1 commit into from
Sep 12, 2019

Conversation

diesalbla
Copy link
Contributor

@diesalbla diesalbla commented Sep 10, 2019

Continuation of #1598, which was automatically closed by Github due to the deletion of the series/1.1 branch. In what follows is the original message, and some items from the conversation:


This unifies the two ADT in the internal stream implementation: FreeC[F[_], R], the Free monad with catch and interruption; and Algebra[F[_], O, R], for the scope-handling and output instructions. These are combined using the approximate type aliae of PullF, O, R] as FreeC[Algebra[F, O, ?], R]. Notably, an object of type Algebra[F, O, R] can only appear in a FreeC through the FreeC.Eval class. This allows us to unify these two ADTs by doing the following:

  • Add an output parameter O (stream output) to the FreeC ADT, and its subclasses; as well as to the ViewL trait (for unrolling).
  • Turn FreeC.Eval into an abstract class that replaces the previous Algebra trait, so that all subclasses of Algebra become subclasses of FreeC.Eval. Because FreeC and Algebra ADTs are in separate files, we unseal it.
  • We replace the ViewL subclasses from containing a F[R] to containing an Eval[F, R].

As a result, a Pull is no longer an alias for FreeC[Algebra[F, O, ?], Unit], but for FreeC[F, O, R] instead.

With the previous 2-layered representation, there was a need to have in the Algebra object a few methods to lift simple data and fill the type parameters. With the new representations, those methods are not needed and we can remove them in favour of the object they build. That is the case of the private methods openScope and step.

Translate to mapOutput

Before this change, the implementation of Stream[F, A].map[B](f: A => B) had to work at two levels: one being a FunctionK between two type-instances of Algebra, which would differ on the output type. To apply this transformation through the Stream, the FreeC ADT had a translate method. After the changes, now we do not need that FunctionK wrapper, and we can just replace the translate method of FreeC with a mapOutput method.

Modularity

The new design keeps some of the modularity of the previous one: the FreeC main classes and operations are built without any reference to the Algebra classes. The major control-flow operations, such as the FreeC methods and the loop unrolling of ViewL, are also implemented without reference to the Algebra object. So, the separation of concerns still exists between these files, except that now it is connected through sub-classing rather than through a parametric type.

Readability

The new representation has no nested types, no kind-projection ? symbol, fewer "smart constructors", and implements stream output map without using a FunctionK. Also, now the F[_] parameter refers everywhere to the effect in which the stream is run. Against those upseides, there are some downsides in adding an extra type-parameter O, and the use of unsealed traits.

Benchmarking

As an optimistic assesment, since this PR reduces the number of objects neeeded to represent each Algebra operation (by no longer needed the FreeC.Eval wrapper), this may significantly reduce memory consumption. An early benchmarking, as mentioned in the initial PR, suggested a reduction of 2.5% memory consumption.

After-work: unify FreeC and Pull

Now that FreeC has two covariant parameters, the only major difference with the definition of Stream and Pull is that the F[_] parameter is covariant in those but invariant in FreeC. I am experimenting with how to add that covariance to FreeC, but at present it is causing not only some compile errors, but also compiler crashes. This PR, however, seems to be stable enough and carries enough changes to be dealt with separately.

This commit unifies the two core ADT of fs2: FreeC and Algebra.
FreeC is a free monad with catch and interruption.
Algebra represents a set of "abstract instructions" that may output
values of type O, or put a value of type R on a stack.
A Stream[F, O] consists of a `FreeC[Algebra[F, O, ?], Unit]`.

We have noted two interesting facts of this combination:
- In fs2, a FreeC was only used when its F[_] type was an Algebra
- The only place in which an Algebra object would appear in a FreeC
  was through the Eval class.

Based on these facts, and to simplify and reduce memory consumption
(as well as some complexity) of the Stream implementation, we
unify FreeC and Algebra, but turning "Algebra" into a subset of FreeC:

- We add to FreeC a third parameter, where the second parameter
  represents the output of a Stream.
- We turn the "Eval" class of FreeC into an abstract trait,
- We remove the Algebra trat, and turn all the former Algebra
  sub-classes into sub-classes of that Eval.

This change causes other changes:

- The `translate` function of the FreeC.Bind is no longer needed. This
  was needed to implement `mapOutput` as a natural transformation.
  Instead, we add the `mapOutput` function to the FreeC itself.
- We introduce another middle sub-class of FreeC, for the results.
  This is just a mere convenience, to reduce code elsewhere.
- Since classes and types are simplified, some factory methods or
  "smart constructors", needed to lift through types, are not needed.
  Such as Algebra.step, openScope, closeScope.
- We are now able to replace the second parameter "Nothing" in the
  inner wrapped FreeC of Pull and Stream (the output) by the O
}
}
): FreeC[F, O, R] =
Acquire(resource, release)
Copy link
Contributor Author

@diesalbla diesalbla Sep 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mpilquist For now, the branch has only unfolded/removed the factory methods that were private to this object. However, since the whole Algebra is private to fs2, would it make sense to open the visibility of the Algebra subclasses, and remove this method, as well as eval and output above, and getScope below?

@diesalbla
Copy link
Contributor Author

diesalbla commented Sep 12, 2019

The attached file contains (compressed) a spreadsheet with the results of some more benchmarking results.

I used the suite StreamBenchmarks with some changes: I reduced the generated range @GenerateN from 1, 10, 100, 1000, 10000, 100000 to just 10, 1000, and 100000. Here is a patch file with the changes. I run these benchmarks, both in the master branch and in this branch, using the command:

jmh:run -f 1 -i 10 -wi 5 .*StreamBenchmark*. -prof gc

The results of these runs were tabulated into this spreadsheet, in which I also add the cells to highlight the relative improvement (the master measures to the left, this PR to the right).

The results indicate, across most benchmarks, a reduction in memory consumption (as measured by the gc.alloc.rate.norm) of 2.5% (the eval benchmark), 3% (leftAssocConcat), 5% (leftAssocFlatmap), 3.25% (rightAssocConcat), and 5% (rightAssocFlatmap). The one micro-benchmark for which no improvement is observed is in toVector, but that is understandable given that most allocations there go in creating the Vector itself.

For the environment:

$ java -version
openjdk version "12.0.1" 2019-04-16
OpenJDK Runtime Environment AdoptOpenJDK (build 12.0.1+12)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 12.0.1+12, mixed mode, sharing)

@pchlupacek
Copy link
Contributor

@diesalbla seems promissing, thanks for this

@mpilquist
Copy link
Member

Looks good to me. Looking forward to unifying FreeC and Pull in to a single type too.

@mpilquist mpilquist merged commit c3cd7df into typelevel:master Sep 12, 2019
@diesalbla diesalbla deleted the unify_algebra_freec branch September 12, 2019 13:21
@mpilquist mpilquist modified the milestones: 2.0.0, 2.0.1 Sep 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants