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

Refactors #884

Merged
merged 30 commits into from
Mar 15, 2021
Merged

Refactors #884

merged 30 commits into from
Mar 15, 2021

Conversation

Anton-Latukha
Copy link
Collaborator

@Anton-Latukha Anton-Latukha commented Mar 12, 2021

Reducing some current YAGNIs.

Starting reducing some superfluous things in the code (#879)

  • Result type is really an euphemism to Either. Left Right is on the instinctive understanding for Hakellers, and using it enables further readability and plasticity to the code.
    Currently made Result, what it is - an euphemism to Either. Probably we may even just remove the custom type without any detriment, it probably would just make code even easier to read, since the Either semantics already fully embody what Result is, and the habitat of the function that brings Either shows where it came from. Outside of the module that exposes the data type Result is literally does not appear not once in type signatures anywhere over the project.

  • forceEff changed a little, no reason to lock and then do preparation actions that do not required locking the whole process. So now doing preparing first and then locking right when it is needed.

  • Please note that force{,Eff} implementations internally are now the same.

  • queryM became non-locking, since read from static structures in a static language does not require locking operations.


Update:

  • notice the implementation transformation for further - now it really shows the nature that all there was - it really simply defers the thunk. The question here is: since locking was not implemented for it, does atomicModifyVar needs thunk locking, or its atomicity allows this. But anyway, further family of functions used just 1-nce in a inform type cass instance that is not used at all.

  • we arrived at that force{,Eff} are exactly 1 function, they are united currently under forceMain, the future (a cascade other MonadThunk instances above, for example, and future in general) would show does class MonadThunk needs forceEff, or forceEff is just force.

@Anton-Latukha Anton-Latukha force-pushed the 2021-03-11-refactors branch 2 times, most recently from d2131d1 to 2ad1dbb Compare March 12, 2021 15:36
@Anton-Latukha
Copy link
Collaborator Author

I have an idea.

@Anton-Latukha
Copy link
Collaborator Author

Anton-Latukha commented Mar 14, 2021

Well.

Wanted to do something like:

Which would allow force = operateOnThunk . queryM, and implement locking as force = (withLock operateOnThunk) . queryM

But readVar ref requires MonadTrans.

-- (roughly)
  -- | Non-blocking query
  queryM :: NThunkF m v -> ExceptT (m v) (IdentityT) v
  queryM (Thunk _ _ ref) =
    do
      (\case
        Computed v -> lift $ pure v
        Deferred a -> throwE a
        ) =<< readVar ref

So far not married the types.

And because of MonadTrans requirement, the result would be hardly faster or better than the current code.

I am still uncomfortable with MonadTrans, though I can construct and operate on them slowly, but the added additional hassle, code noisiness & complexity, etc. they do not smile to me. So I'm being lazy with remembering them keep forgetting them and as lazy re-learning them or touch them.

I know that what uses State monad with enough (sometimes big) persistence - that the data processing code can be written properly elegantly without it, but short-circuiting the I to O response to help debug the process may take some time.

There is a continuation-passing style.

I just do not love MonadTrans for so many reasons, being so very lazy with it.

@Anton-Latukha Anton-Latukha force-pushed the 2021-03-11-refactors branch 6 times, most recently from 5c49502 to b45f208 Compare March 15, 2021 00:06
For the `if` FP programmers always internally psychologically ask `& else?`.
And since this functions implicitly return as an else a stub instead of
processing, for what `when*` is used in the language - fits better.
Implementation show that we so far not needed the custom type for it. Right and
Left already embodies the whole semantic meaning, since Either is mostly used as
Right for computations being successful, and Left for failure. So why bother
everyone with remembering the custom constructors and what they are for if they
are not needed, Either is already know to everybody.

Current implementation by itself shows that `Result` custom type is being YAGNI,
since replacing it with Either changes nothing.
So it relates to the topic of #879.

If we would need ad-hock abilities to "our custom class" - the
`TypeSynonymInstances` are there for us.

I did the main work abstracting implementation from the type and constructors
here, so if we would want in the future to change the type - we would just need
to replace the `either` function with `result` function and that is all.

But just using `either` is more consise.

Type synonym left the signature information there. I'd even considered to remove
`Result` all togather, since the more I talk about it the more it becomes clear
that its use is just what Haskellers use `Either` for.
Notice, here in `forceEff` I moved the thunk locking closer to where it is
needed, so function does a couple of preparation computations before locking the
thunk to operate on it.

Also notice that the `force` add `forceEff` look like 1 function, and as
`forceEff` also probably needs to check for exception when doing an action -
they are literally the same function.
So far `queryM` is not used. And locking the thunk computation on static-copying
structures to check if variable is computed is weird. Lets allow reading from
static structure until we would need locking during reads.
This is was was possible to replace right away in-place.

Also added `MonadFail` requirement to `MonadDataErrorContext t f m`, as it was
obvious addition to allow to demote some `errors` into `fail`.

Reference:
https://www.haskellforall.com/2019/12/prefer-to-use-fail-for-io-exceptions.html
Seems logical that `forceEff` needs to have the exception catcher on `action`
and "Loop detected" is just a ThunkLoop.

So now `forceEff == force`
Now it shows that `further` body is actually a `const`.
We saw that `further` does not even need to strictly pattern match on a thunk,
since the results of bothe matches would be the same, so we can skip matching.
Just organizing and layouting
@Anton-Latukha
Copy link
Collaborator Author

In fact, talking about the continuation-passing style, instances I was refactoring were created with it.

So after creation, they were needed to be refactored inside-out to achive optimization.

layus added a commit to layus/hnix that referenced this pull request Dec 17, 2021
…nim of Either; Thunk.Basic: nonblocking queryM, internal changes to `force*`
layus added a commit to layus/hnix that referenced this pull request Dec 17, 2021
…m of Either; Thunk.Basic: nonblocking queryM, internal changes to `force*`
layus added a commit to layus/hnix that referenced this pull request Dec 17, 2021
…nim of Either; Thunk.Basic: nonblocking queryM, internal changes to `force*`
layus added a commit to layus/hnix that referenced this pull request Dec 17, 2021
…m of Either; Thunk.Basic: nonblocking queryM, internal changes to `force*`
layus added a commit to layus/hnix that referenced this pull request Dec 17, 2021
…nim of Either; Thunk.Basic: nonblocking queryM, internal changes to `force*`
layus added a commit to layus/hnix that referenced this pull request Dec 17, 2021
…m of Either; Thunk.Basic: nonblocking queryM, internal changes to `force*`
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.

2 participants