RFC: Speed up partitions(n), partitions(n,m) iteration

[kmsquire]

• Reuse output array
• Specialize IntegerPartitions, FixedPartitions based on whether or not to copy
• Specialize collect() to always copy
• Reduce memory allocation

The speed of collect() hasn't changed much, and iterating with for is about 4x as fast as using collect()

I was wondering if I could specialize types using Bools directly (you can't), so I'm specializing using

partitions(n::Int; copy::Bool=false) = IntegerPartitions{int(copy)}(n)

[JeffBezanson]

I should allow Bools as type parameters.

I think the COPY type parameter is a pretty awkward way to get performance. We need to improve the GC and not bother with things like this. Since iteration should be purely functional, a better API for this would be to explicitly call a function nextpartition!(n, as).

[kmsquire]

@JeffBezanson, this was somewhat in response to @StefanKarpinski's comment in https://groups.google.com/forum/#!searchin/julia-users/counter$20collect/julia-users/b_W8Do8PxJw/nK-nNoYVDDUJ, regarding the comparison of the Counter class type in @timholy's Grid.jl package (which reuses the same array for iteration) to the current implementation of, e.g., partitions, which explicitly creates a new iteration array.

Since iteration should be purely functional, a better API for this would be to explicitly call a function nextpartition!(n, as).

Okay, nextpartition(n,as) in this patch should be called nextpartition!(n, as), since it does modify the iteration variable. I can fix this.

Is it really possible to improve the GC so that explicitly allocating an iteration array each iteration (which the current code does) is as fast as modifying the same array, as is done in this patch?

[StefanKarpinski]

Matlab's JIT does often manage to do just that (the pressure to do this in Matlab is much greater since you can't mutate arrays in place except by dropping into C). I believe the plan is to use a bit in the type tag to do approximate reference counting, coupled with some escape analysis to determine in many cases that memory can be reused safely. See #261.

[JeffBezanson]

I would say mutating the iteration state is somewhat ok, but not an array
that's returned to the user as a generated value. If those two arrays are
the same, you're out of luck.
On Aug 24, 2013 2:15 PM, "Stefan Karpinski" notifications@github.com
wrote:

Matlab's JIT does often manage to do just that (the pressure to do this in
Matlab is much greater since you can't mutate arrays in place except by
dropping into C). I believe the plan is to use a bit in the type tag to do
approximate reference counting, coupled with some escape analysis to
determine in many cases that memory can be reused safely. See #261#261
.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/4136#issuecomment-23213658
.

[kmsquire]

I think the COPY type parameter is a pretty awkward way to get performance. We need to improve the GC and not bother with things like this. Since iteration should be purely functional, a better API for this would be to explicitly call a function nextpartition!(n, as).

I've renamed the functions. What do you recommend with this patch, then? Some options include

1. remove the type parameter, have it use the same return array for each iteration, but specialize collect to copy the array at each iteration
2. the patch is fine as is
3. drop the patch; the current implementation of creating a new array on every iteration is the most consistent behavior, and improvements in the GC will make these kinds of shenanigans unnecessary.

I'm fine with any of these.

[JeffBezanson]

Unfortunately 3 is the only option. If we did 1, we would have to specialize untold other functions as well.

[timholy]

@kmsquire, sorry if I egged you on in an unproductive direction.

[kmsquire]

No worries! There are actually a couple of small optimizations that I discovered while doing this that still work with the current behavior. I'll submit those separately.