Skip to content

user-defined eagerization #8

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

Closed
wants to merge 4 commits into from

Conversation

kurt-o-sys
Copy link

implement both multimethods and fn-passing to provide a user-defined eagerization

+ rename manage-with-courage -> manage-eager-conditions
+ add manage-with function
@kurt-o-sys
Copy link
Author

kurt-o-sys commented Jul 24, 2017

Answering: #2 (comment)

Making manage-with-courage public may help people out to write an own version of manage. I guess adding some examples in the documentation would be nice in that case. However, if the same (alternative) implementation is used often, adding the choice of eagerization to the library makes a lot of sense to me. This prevents that everyone starts to write the same manage-function(s) in their own code base.

As different implementations, there seem to be a few strategies already mentioned:

  1. pr-str - I fail to see why this is a good choice, but the author does, and that's good enough
  2. walk/postwalk - efficient, fast, I like that one (it does some eagerization, but fails in some weird cases, just like pr-str)
  3. identity - no eagerization if you don't use laziness (or don't care about weird behavior when it comes to lazy data structures)

There may be other strategies that make sense, but with these 3, I guess about 99% of the cases will be covered. So one might opt for a cond instead of multimethod, adding new ways of eagerization to the library as it's requested. However, I don't know why one would 'close' it.

@clojureman
Copy link
Owner

clojureman commented Jul 24, 2017

I am a bit afraid of allowing the eagerization to be mutated by the user of the library.
This is because if two libraries each set their own custom eagerization strategy for this lib, they can potentially become incompatible with no warning to the user

@kurt-o-sys
Copy link
Author

Well... it's in the manage-function that the user decides which strategy to use, right? If another library, let's say A, uses a certain strategy inside library A, the user of library A shouldn't know or care about this. It's up to the maintainer of library A to make the right decision and document it properly.

However, if library A also adds the special condition system, as a user of A, I'd prefer to add my eagerization of preference. The maintainer of library A doesn't know about the data structures I'm using. I do, however, so it's my call to make the right decision.

Moreover, making manage-with-courage public will result in exactly the same 'potential incompatibility', won't it?

@kurt-o-sys
Copy link
Author

kurt-o-sys commented Jul 24, 2017

Oh, you're talking about a global setting? <- #2 (comment)

You certainly misunderstood! I am not in favour of a global setting at all! I am in favour if a setting inside the manage-function. Please, no global settings... I was at any point referring to something like the manage-with and manage-as functions (see pull request)... The former passes functions, the latter is multimethod based. Both allow for something like (def manage (partial manage-with ...)) (as I always intended).

Where does the global setting idea suddenly comes from?

@didibus
Copy link

didibus commented Jul 26, 2017 via email

@kurt-o-sys
Copy link
Author

kurt-o-sys commented Jul 26, 2017

That's exactly what I'm trying to say (I think).
pr-str doesn't guarantee eagerization, similar to post-walk. I could only see 1 case in which the latter doesn't work, and its in the case I've shown in #2 (comment) (which I call 'embedded laziness' further down).
Again, I don't think pr-str has many advantages over postwalk, and both don't guarantee eagerization. That's why I added 3 strategies now, open to adding new ones (using multimethods). There might be a 4th, like 'expensive'. It's pretty easy to document it. Basically, it comes down to:

  1. Do you use laziness?
    • no, use identity
    • yes, go to step 2
  2. Do you have 'embedded laziness' (or are you not sure)
    • yes - well, expensive eagerization might solve that case, if you don't take care of it yourself
    • no - post-walk will do

I don't see a use case for pr-str, as stated already a few times, but some people seem to do, so that's why I keep it in the loop :). An example of when pr-str fails, see the case I've shown above. I'm not sure if there's much difference between pr-str and postwalk - that's why I asked for the use cases where these fail. Again, to me, both fail to guarantee eagerization, so they score about equal to me on that criterium. postwalk wins when it comes to performance... There's no use case for pr-str I can think of.

That being said, I certainly agree there shouldn't be more than 3 (or maybe 4?) variants in the library and optimizing them over time:

  1. no laziness
  2. realize lazyniness (clojure structures only)
  3. realize everything, incl. laziness embedded in java objects (will need reflection somehow)

I suppose these would cover about 99.9% of the cases. So, coming back to your proposition of 2 strategies, well, I agree - I might go for 1 or maybe 2 more :p. The difference is that one uses different functions, the other the same functions with different parameters.
I only submitted both manage-with and manage-as to show some different strategies. I certainly don't think both are necessary or even a good idea. I just want to be able to use 'no eagerization' (in the case I know I don't use laziness) and 'clojure eagerization' (using post-walk, not pr-str).

@didibus
Copy link

didibus commented Jul 27, 2017

Ya, I'm not personally seing value in having more then one eagerization strategy. This lib is for conditional restarts, not eagerization. But I'll let the maintainer make that choice, obviously if others see value for them, I'll still be able to use it for my needs.

The fact there's no guaranteed mechanism for eagerization in Clojure, I'd almost prefer that only a non eagerizing manage existed. Make it very explicit to users that they must design for laziness when they are using laziness. As I said before, even Clojure's try/catch block does not handle laziness.

@kurt-o-sys
Copy link
Author

Right, I follow that line of thought... and I fully agree on documenting - I've been stating that already a few times as well. However, if there's only 1 strategy, it shouldn't be pr-str, which is inefficient and doesn't provide any guarantees of eagerization. That was how this discussion started - the pr-str.

It seems better to me to document in which cases eagerization doesn't work (with pr-str, that's a bit vague) and use an efficient eagerization. Or just drop any eagerization and leave it fully to the user. I can live with that as well. But having pr-str as the default and the only strategy, it doesn't seem a good idea to me at all. Some people seem to like the pr-str-strategy, some obviously don't. Having a more pluggable eagerization, or at least to avoid pr-str didn't, and still doesn't seem like a bad idea to me.

, I'd almost prefer that only a non eagerizing manage existed.

Right! Avoid pr-str

It has been stated that one can fork and start changing the strategy, but please refer to #2 . I don't think that's a good idea either. I'd prefer to have 1 library that fits most uses, not having a bunch of forks all doing the same, but with only 1 or 2 lines of code difference.

@didibus
Copy link

didibus commented Jul 27, 2017

I agree, pr-str is hacky. It relies on hoping that each element at level 0 will have an implementation of print which will loop further down one level.

The hacky advantage of pr-str is that some implementation of toString on Java types tend to do that also.

Maybe instead of (constantly nil), you could have have:

(cond
  (.startsWith (class %) "java.lang") nil
  :else (pr-str %))

This would make it so we still bank on pr-str for java types, but just walk everything that's Clojure or primitive.

In the future, we could even had more concrete java types to cond and have more optimized walkers for them.

@kurt-o-sys
Copy link
Author

kurt-o-sys commented Jul 27, 2017

right... but why not using post-walk for clojure structures? More like:

(cond
  (.startsWith (class %) "clojure.lang") (walk/postwalk (constantly nil) %) ;; still no guarantees here! - check my example
  :else  (pr-str %) )

@didibus
Copy link

didibus commented Jul 29, 2017

Ya, that can work too. Basically just a hybrid approach, so it uses postwalk when it can, and falls back to pr-str when it can't. So we get speed and we don't regress on coverage.

I was thinking, if you look at the postwalk source, that we can do the same, just where they have :else, we would habe it call pr-str, unless it was a primitive from java.lang.

@didibus
Copy link

didibus commented Jul 29, 2017

(cond
   (list? form) (outer (apply list (map inner form)))
   (instance? clojure.lang.IMapEntry form) (outer (vec (map inner form)))
   (seq? form) (outer (doall (map inner form)))
   (instance? clojure.lang.IRecord form)
     (outer (reduce (fn [r x] (conj r (inner x))) form form))
   (coll? form) (outer (into (empty form) (map inner form)))
   :else (outer
            (cond
              (.startsWith (class form) "java.lang") form
              :else (do (pr-str form) form))))

Something like that.

@didibus
Copy link

didibus commented Jul 31, 2017

I implemented what I was suggesting: #9

@kurt-o-sys
Copy link
Author

see #9 :)

@kurt-o-sys kurt-o-sys closed this Jul 31, 2017
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