Fork me on GitHub
#clojure-dev
<
2019-08-07
>
murtaza05:08:31

I've created a patch to protocolize the creation of zippers. I've basically built a zip2 entirely on top of zip. This approach was taken to ensure that I don't end up removing the existing counterpart functions from zip, since that might be a breaking change. Curious to know if anyone has a different approach in mind for this. Here's the ticket along with the patch https://clojure.atlassian.net/projects/CLJ/issues/CLJ-2530?filter=allopenissues&amp;orderby=created%20DESC

hiredman05:08:32

Did this get discussed on the mailing list at all?

hiredman05:08:40

The ticket doesn't really convey a strong sense of what is being solved to me, so I wonder if I am missing some context

alexmiller05:08:02

zip would absolutely have been a better design with protocols (if protocols had existed when it was created)

alexmiller05:08:11

I've written this same thing before

alexmiller05:08:34

but just because we can do it is not a strong argument for why we should do it

alexmiller05:08:05

perf is one possible reason

alexmiller05:08:30

oh, looking at the patch you're actually cutting a different dimension than I thought

alexmiller05:08:57

zippers themselves are a collection of 3 functions and I thought you were creating a protocol for that

murtaza05:08:06

That's what I've done, haven't I?

alexmiller05:08:06

you've got all the functions buried under one protocol method

alexmiller05:08:28

I expected something like

(defprotocol Zipper 
  (branch? [this node]) 
  (children [this node]) 
  (make-node [this node children]))

alexmiller05:08:30

tying those impls to concrete types is bad b/c you might want different functions for the same type

alexmiller05:08:01

if dispatching on the concrete type was the idea here, I don't think it's a good one

murtaza05:08:30

Is there not being an easy way to protocolize without creating a breaking change what makes the argument for this change less compelling?

hiredman05:08:06

Yeah, that is the other thing "protocolizing zippers" can mean different things, and this approach is the one I would expect the least

alexmiller05:08:24

yes, looking at the patch, I was surprised

murtaza05:08:39

What would be a better approach?

hiredman05:08:41

You might check out something like https://github.com/akhudek/fast-zip

alexmiller05:08:33

even that is not what I'd expect :)

hiredman05:08:33

Although I guess that is all deftypes, no protocols

hiredman05:08:53

I feel like maybe bbloom had something

alexmiller05:08:56

but the argument being made in that is perf

alexmiller05:08:59

I wrote one way back when I wrote the zipper article in 2010/2011, but doesn't look I actually made a repo out of it

alexmiller05:08:39

but to unroll back to a good question - what problem are you trying to solve?

alexmiller05:08:52

what does this version do that the current version doesn't?

schmee18:08:59

can Clojure loop constructs be unrolled by the JIT under any circumstance?

alexmiller18:08:11

they're pretty much the same as Java loops in the bytecode

schmee19:08:26

I thought the JIT only unrolled counted loops (like for with a constant stride)?

hiredman19:08:56

the jit only sees bytecode, for loops don't exist in the bytecode

hiredman19:08:15

so what the jit can unroll are particular patterns of jumps in the bytecode

schmee19:08:09

this might be nonsense but I’ll ask anyway: Maurizio Cimadamore mentioned in the Panama update from JVMLS that most loop optimizations don’t apply when using longs, am I correct that this would be an issue for Clojure loops as well since all primitive math is done with longs? https://youtu.be/r4dNRVWYaZI?t=2100

schmee19:08:10

hah, it actually does seem to be the case: I have a Java loop with that gets unrolled when the loop variable is an int, but not when it’s a long, and the equivalent Clojure loop also does not get unrolled