GroupByUntil with maxGroups

[akarnokd]

Split from #641

Purpose: limit the active groups to a certain number. If new group is opened, the oldest group is closed.

[cloudbees-pull-request-builder]

RxJava-pull-requests #600 FAILURE
Looks like there's a problem with this pull request

[headinthebox]

Why not remove the one that got a notification last? Not sure if this specific behavior particularly useful.

[akarnokd]

I don't know. I think it matches Rx.NET's signature but I can't really look into ConcurrentDictionary. It is possible the capacity is just for the initial capacity of the map and has nothing to do with limiting the groups.

[benjchristensen]

This doesn't match the Rx.Net signature: http://msdn.microsoft.com/en-us/library/hh211932(v=vs.103).aspx

public static IObservable<IGroupedObservable<TKey, TSource>> GroupByUntil<TSource, TKey, TDuration>(
    this IObservable<TSource> source,
    Func<TSource, TKey> keySelector,
    Func<IGroupedObservable<TKey, TSource>, IObservable<TDuration>> durationSelector
)

Why is maxGroups being added to the signature?

[akarnokd]

I saw this added in Rx 2.2.2: http://rx.codeplex.com/SourceControl/latest#Rx.NET/Source/System.Reactive.Linq/Reactive/Linq/Observable/GroupByUntil.cs

I thought it was about limiting the groups, but apparently the ConcurrentDictionary uses the capacity to specify the concurrency level, not the group number.

I guess this PR can be ignored.

[benjchristensen]

@akarnokd Since we already have groupByUntil in Observable, what is the reason for adding more overloads with maxGroups? Where is this use case and signature coming from?

The stated reason is:

Purpose: limit the active groups to a certain number. If new group is opened, the oldest group is closed.

However, that is an arbitrary decision on how to "clear out the cache" and one that would need different eviction policies. If this is going to be pursued it needs something more thorough than just a maxGroups.

As for ConcurrentDictionary limiting groups, there is no evidence to that from looking at the API. It seems it will keep growing, at least to the max Integer size for number of keys. Also, ConcurrentDictionary is a implementation detail, not part of the groupByUntil contract.

Is it correct that you're trying to create a groupByUntil variant that provides a cache eviction policy? If so, why, and how should it behave if the key shows up again?

[benjchristensen]

Closing based on discussion above ...

I guess this PR can be ignored.