Jake — We're not constructing a new object each time a function is invoked
because the "decorated" functions are only created when a function is
registered, but we'll keep your concern in mind.
Thanks for all of the feedback from everyone. I think we have the
information we need to move forward. In summary, we'll have to change our
implementation such that the methods in FunctionService still return the
same objects they did before our changes.
- Aaron
On Mon, Sep 16, 2019 at 7:54 PM Jacob Barrett <jbarr...@pivotal.io> wrote:
> Sorry I am entering this late in the game but why do we need to decorate
> the function at all for metrics. The current implementation has statistics
> without decoration. All the concerns raised in this thread concern me as
> well as the cost of constructing yet another object every time a function
> is invoked by serialized function, Execution.execute(Function). Each
> allocation puts more pressure on the GC, decreases our throughput and
> increases our latency.
>
> -Jake
>
>
> > On Sep 11, 2019, at 2:29 PM, Dale Emery <dem...@pivotal.io> wrote:
> >
> > The stats use the ID of the function, and each TimingFunction reports
> the same ID as the function it wraps. So I think the stats would look like
> they always did.
> >
> > Dale
> >
> > —
> > Dale Emery
> > dem...@pivotal.io
> >
> >
> >
> >> On Sep 11, 2019, at 12:14 PM, Anthony Baker <aba...@pivotal.io> wrote:
> >>
> >> I think the Decorator approach you outlined could have other impacts as
> well. Would I still be able to see specific function executions in
> statistics or would they all become “TImingFunction”?
> >>
> >> Anthony
> >>
> >>
> >>> On Sep 11, 2019, at 12:00 PM, Aaron Lindsey <alind...@pivotal.io>
> wrote:
> >>>
> >>> Thanks for your response, Dan.
> >>>
> >>> The second scenario you mentioned (i.e. users calling
> >>> FunctionService.getFunction(String)) worries me because our PR changes
> the
> >>> FunctionService so they would now get back an instance of the decorator
> >>> function instead of the specific instance they registered by calling
> >>> FunctionService.registerFunction(Function). Therefore, any explicit
> casts
> >>> to a Function implementation like (MyFunction)
> >>> FunctionService.getFunction("MyFunction") would fail. Does that mean
> this
> >>> be a breaking change? The FunctionService class does not specify that
> >>> getFunction must return the same type function as the one passed to
> >>> registerFunction, but I could see how users might be relying on that
> >>> behavior since there is no other way to get a specific function type
> out of
> >>> the FunctionService without doing a cast.
> >>>
> >>> - Aaron
> >>>
> >>>
> >>> On Wed, Sep 11, 2019 at 10:52 AM Dan Smith <dsm...@pivotal.io> wrote:
> >>>
> >>>> Functions are serialized when you call Execution.execute(Function)
> instead
> >>>> of Execution.execute(String). Invoking execute on a function object
> >>>> serializes the function and executes it on the remote side. Functions
> >>>> executed this way don't have be registered.
> >>>>
> >>>> Users can also get registered function objects directly from the
> function
> >>>> service using FunctionService.getFunction(String) and do whatever
> they want
> >>>> with them, which I guess could include serializing them.
> >>>>
> >>>> Hope that helps!
> >>>> -Dan
> >>>>
> >>>> On Wed, Sep 11, 2019 at 10:27 AM Aaron Lindsey <alind...@pivotal.io>
> >>>> wrote:
> >>>>
> >>>>> As part of a PR to add Micrometer timers for function executions
> >>>>> <https://github.com/apache/geode/pull/4038>, we implemented a
> decorator
> >>>>> Function that wraps all registered non-internal functions and adds
> >>>>> instrumentation. This PR is
> >>>>> failing AnalyzeSerializablesJUnitTest.testSerializables because the
> >>>>> decorator class is a new Serializable.
> >>>>>
> >>>>> I'm not sure if it would be OK to add this class to
> excludedClasses.txt
> >>>>> because I don't know whether this function will ever be serialized.
> If it
> >>>>> will be serialized, then I'm concerned that this might break
> backwards
> >>>>> compatibility because we're changing the serialized form of
> registered
> >>>>> functions. If this is the case, then we could implement custom logic
> for
> >>>>> serializing the decorator class which would replace its serialized
> form
> >>>>> with the serialized form of the inner class. Again, I'm not sure if
> that
> >>>>> would be necessary because I don't know the conditions under which a
> >>>>> function would be serialized.
> >>>>>
> >>>>> Could someone help me understand when functions would be persisted or
> >>>> sent
> >>>>> over the wire so I can determine if this change would break
> >>>> compatibility?
> >>>>>
> >>>>> Thanks,
> >>>>> Aaron
> >>>>>
> >>>>
> >>
> >
>
>
