Skip to content

Confusion with Reducer.Count(), memoization vs mutablity #453

@mgravell

Description

@mgravell

When using reducers, you might use (for example) Reducer.Count() to fetch the count. If you want to alias that to an explicit field, you might use reducer.As("my_alias"). Unfortunately:

  • Reducer.Count() is memoized and singleton, with a private .ctor
  • the aliasing via .As() treats it as mutable
  • there is also a .Alias with a setter

This means that competing uses smash into each-other, and randomly change the alias of unrelated queries, for example:

// query one
search.GroupBy(..., Reducers.Count().As("qty"))...

// unrelated, query two, could be on a different thread
unrelatedSearch.GroupBy(..., Reducers.Count().As("count"))...

Who gets qty and who gets count is effectively random at this point.

Related code:

internal static readonly Reducer Instance = new CountReducer();

This could even break arg composition in niche race cases when changing between aliased and non-aliased.

Options:

  1. remove the memoization, so each .Count() returns a different instance
  2. remove the assumption of safe mutability, so that:
  • .As(...) returns a different instance when the value changes
  • the .Alias setter throws an exception (we can also mask the setter via property overloading)

Option 1 is simpler and risk free, but has the negative side-effect is performance (more allocations)
Option 2 is more complex and risks runtime breaks

However, if people are using aliasing (and they probably should be), option 2 would end up with the same allocations anyway, so: my vote goes "keep it simple, option 1".

Any thoughts @uglide @atakavci ?

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions