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

Add Concurrency Basics section #287

Merged
merged 9 commits into from Aug 20, 2018
Merged

Add Concurrency Basics section #287

merged 9 commits into from Aug 20, 2018

Conversation

Avasil
Copy link

@Avasil Avasil commented Jul 24, 2018

I've written something regarding Concurrency and I hope it could be good addition to documentation. :)

My idea is to gather all relevant information that is usually scattered around the internet, books and gitter conversations. With your review/contributions I believe it will become decent reference point for understanding basics of concurrency that are relevant to Cats-Effect and others.

As time passes and I understand more I'll try adding more information - I want to add short "Stack safety" section next but I scrapped it for now to do this PR and release anything.

If you like this initiative we could make a TODO list and have corresponding issues to gradually make it more comprehensive.

It's probably the first time I'm writing something that long in English so I will really appreciate any suggestions regarding wording, style etc. so the message is clear. Also most of the content is my own understanding so it might be completely wrong. :P

too many threads which are waiting for callback from other (blocked) thread for a long time we risk
getting deadlock that prevents any new tasks from starting their work.

#### Fork Join Pool
Copy link
Author

@Avasil Avasil Jul 24, 2018

Choose a reason for hiding this comment

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

I would like to have it in first version but I haven't been able to find any decent sources about it in context of Future, IO, Task etc.

Does anybody have explanation what are benefits of fork join pool (which is the default) vs regular fixed thread pool? How is it possible to split IO?

@codecov-io
Copy link

codecov-io commented Jul 24, 2018

Codecov Report

Merging #287 into master will increase coverage by 3.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #287      +/-   ##
==========================================
+ Coverage   85.98%   89.04%   +3.05%     
==========================================
  Files          63       68       +5     
  Lines        1713     1761      +48     
  Branches      177      187      +10     
==========================================
+ Hits         1473     1568      +95     
+ Misses        240      193      -47

val ecOne = ExecutionContext.fromExecutor(Executors.newSingleThreadExecutor())
val ecTwo = ExecutionContext.fromExecutor(Executors.newSingleThreadExecutor())

def infiniteIO(id: Int)(implicit ec: ExecutionContext): IO[Fiber[IO, Unit]] = {

Choose a reason for hiding this comment

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

Per discussion in the Gitter channel: I think it might also help to clarify the purpose of the implicit ec: ExecutionContext parameter of infiniteIO. As far as I understand, it's used by repeat.start. So when you call prog.unsafeRunSync(), what's happening with ec while ecOne and ecTwo do all the work?

Copy link
Member

Choose a reason for hiding this comment

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

@Avasil the API on master is going to change thanks to #289 — you should not require an ExecutionContext here, but a ContextShift.


Essential term is **thread scheduling**. Since we can’t run all our threads in parallel all the time they
each get their own slice of time to execute, e.g. interleaving with the rest of them so every thread has a chance to run.
If there is a time for change the currently running thread is **preempted** - it saves its state and context switch happen.
Copy link
Contributor

Choose a reason for hiding this comment

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

Awkwardly worded. Potential fix: If it is time to change threads, the currently running thread is preemted - it saves its state and the context switch happens.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you @gatorcse !

@alexandru
Copy link
Member

@Avasil I'm striving to get all PRs for RC3 merged, so if there's anything else you want to do with this document, today would be a good time, otherwise I'll merge as is.

The website doesn't have the same requirements as code, so no pressure, we can merge or do other corrections later.


It's about time to get it right:
```scala
def infiniteIO(id: Int)(implicit ec: ExecutionContext): IO[Fiber[IO, Unit]] = {
Copy link
Member

Choose a reason for hiding this comment

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

Here too we need ContextShift, not ExecutionContext.

result is available) but it doesn't block threads - other `IO` is free to execute on the thread in the meantime.
This is further explained [in Fabio Labella's comment.](https://github.com/typelevel/cats-effect/issues/243#issuecomment-392002124)

It is important to recognize that not all I/O operations are blocking and need to execute on dedicated thread pool.
Copy link
Member

Choose a reason for hiding this comment

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

Given you've given shift examples in this document, we'll need a sample for ContextShift#evalOn here, since it's meant for blocking operations.

@Avasil
Copy link
Author

Avasil commented Aug 16, 2018

Thanks @alexandru - when I was writing it there wasn't #289 yet, I can update it later today (probably in the evening)

@alexandru
Copy link
Member

@Avasil #289 is now on master.

As I said, documentation doesn't have the same requirements, so no rush, but would be cool to merge this too this week.

will ever run on it!

In other words - `IO` is executing synchronously until we call `IO.shift` or use function like `parSequence` which
does it for ourselves. In terms of individual thread pools we can actually treat `IO` a bit like **green thread** with
Copy link

Choose a reason for hiding this comment

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

Should the green threads be explained somewhere above? Like after Threading (on JVM)?

Before any thread can start doing real work the OS needs to store state of earlier task and restore the state
for the current one. This cleanup has actually nontrivial cost so from performance point of view
the most efficient situation for CPU-bound tasks is when we execute as many threads as the number of available
operating systems' threads favoring synchronous execution whenever possible.
Copy link

Choose a reason for hiding this comment

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

This sentence seems really complex to me. Is it possible to rephrase or split?

@Avasil
Copy link
Author

Avasil commented Aug 16, 2018

@alexandru
I've updated examples for RC3 and addressed all current suggestions. :)

From my side it's good to merge (but I'm still curious about Fork Join Pool for IO :P) if you skimmed through it and didn't notice any glaring errors.

Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

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

There is more we could add and a few things we could debate, but it's a big net positive. I'm in favor of merging this, and we can continue to refine it after RC3.

Thanks, @Avasil, for the hard work.

@Avasil
Copy link
Author

Avasil commented Aug 20, 2018

Thanks for corrections @rossabaker :)

I think I'll make an issue with todo list and maybe it will be good place to give any criticism

@rossabaker rossabaker merged commit e0da607 into typelevel:master Aug 20, 2018
@Avasil Avasil deleted the concurrencySection branch September 8, 2018 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants