You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@isis.apache.org by Erik de Hair <e....@pocos.nl> on 2018/02/12 11:48:03 UTC

Re: Always calling getXxx() and hideXxx() both decreases performance of application (?)

Hi Dan,

As far as I can see the fix [1] for ISIS-1759 [2] is not merged to 
current release of Apache Isis [3]

[1] https://gitbox.apache.org/repos/asf?p=isis.git;a=commitdiff;h=c6c2fc7
[2] https://issues.apache.org/jira/browse/ISIS-1759
[3] 
https://github.com/apache/isis/blob/master/core/viewer-wicket-model/src/main/java/org/apache/isis/viewer/wicket/model/models/ScalarModel.java#L290

We're upgrading to 1.16.0 now but the issue is still there...

Did I miss something or is the issue not fixed yet?

Thanks,
Erik


On 10/26/2017 12:45 AM, Dan Haywood wrote:
> Raised https://issues.apache.org/jira/browse/ISIS-1759 for this
>
> On Sun, 1 Oct 2017 at 19:52 Dan Haywood <da...@haywood-associates.co.uk>
> wrote:
>
>> Hi Erik,
>> I'll look into this... This doesn't sound right that this version is
>> creating these extra calls into the domain code.
>> Thx,
>> Dan
>>
>> On Wed, 27 Sep 2017, 11:35 Erik de Hair, <e....@pocos.nl> wrote:
>>
>>> Hi,
>>>
>>> I'm not really sure about this but it feels like our app's performance
>>> (Apache Isis 1.15.0) is worse than before and I believe it might be
>>> because of the fact that the getter and hide method of a property are
>>> called both, despite the fact that the property is hidden.
>>>
>>> We have a (derived) property (or referenced property) defined like this:
>>>
>>> public String getX(){
>>>       return someService.createX(getY(), getZ()); // We don't want this
>>> method to be called always, because it's an expensive one
>>> }
>>> public boolean hideX(){
>>>       boolean condition = ... // some condition
>>>       return condition;
>>> }
>>>
>>> In 1.14.0 getX() wouldn't be called if the hideX()-method returned true
>>> and so wouldn't the expensive service-method be called. For 1.15.0 we
>>> would have to change the implementation to something like the following
>>> to make sure the expensive call wouldn't be executed:
>>>
>>> public String getX(){
>>>       if(!hideConditionsForX){
>>>           return someService.createX(getY(), getZ());
>>>       }
>>>       return null;
>>> }
>>> public boolean hideX(){
>>>       return hideConditionsForX();
>>> }
>>> private boolean hideConditionsForX(){
>>>       // ... must probably be implemented using QueryResultsCache to
>>> reduce DB-calls
>>> }
>>>
>>> This is also the case for referenced properties for which no DB-query
>>> has to be executed if it was hidden. This could count up if we have a
>>> lot of hidden properties (this might be solved with refactoring).
>>>
>>> Am I seeing this correctly?
>>>
>>> Erik
>>>
>>>
>>>


Re: Always calling getXxx() and hideXxx() both decreases performance of application (?)

Posted by Dan Haywood <da...@haywood-associates.co.uk>.
ok, raised https://issues.apache.org/jira/browse/ISIS-1864


On Sat, 17 Feb 2018 at 15:24 Dan Haywood <da...@haywood-associates.co.uk>
wrote:

> Hi Erik,
>
> Yes, I'm not sure myself what the impact would be.
>
> I think that the parent EntityModel shouldnt' really have this
> responsibility of telling each of its child ScalarModel's to reset
> themselves.  Rather, perhaps it should simply instruct the ScalarModels
> that they are now dirty, so that if and when the ScalarModels are next
> asked for their state (for re-rendering) they then will query the
> underlying domain object ... but not before.
>
> I think I'd rather do this in a separate ticket, though, which I'll raise.
>
> thx
> Dan
>
>
> On Fri, 16 Feb 2018 at 10:18 Erik de Hair <e....@pocos.nl> wrote:
>
>> Hi Dan,
>>
>> We have the same issues on [1] when executing an action.
>>
>> I can create a pull request and add equal code as for the fix on [2] but
>> I can't foresee what a possible impact might be.
>>
>> Thanks,
>> Erik
>>
>> [1]
>>
>> https://github.com/apache/isis/blob/master/core/viewer-wicket-model/src/main/java/org/apache/isis/viewer/wicket/model/models/EntityModel.java#L400
>> [2]
>>
>> https://github.com/apache/isis/blob/master/core/viewer-wicket-model/src/main/java/org/apache/isis/viewer/wicket/model/models/ScalarModel.java#L290
>>
>> On 02/12/2018 04:01 PM, Dan Haywood wrote:
>> > Hi Erik,
>> >
>> > Mistake on my part.  I've brought the ticket back for fix in 1.16.1, and
>> > added a comment to the ticket itself.
>> >
>> > thx
>> > Dan
>> >
>> >
>> > On Mon, 12 Feb 2018 at 11:48 Erik de Hair <e....@pocos.nl> wrote:
>> >
>> >> Hi Dan,
>> >>
>> >> As far as I can see the fix [1] for ISIS-1759 [2] is not merged to
>> >> current release of Apache Isis [3]
>> >>
>> >> [1]
>> https://gitbox.apache.org/repos/asf?p=isis.git;a=commitdiff;h=c6c2fc7
>> >> [2] https://issues.apache.org/jira/browse/ISIS-1759
>> >> [3]
>> >>
>> >>
>> https://github.com/apache/isis/blob/master/core/viewer-wicket-model/src/main/java/org/apache/isis/viewer/wicket/model/models/ScalarModel.java#L290
>> >>
>> >> We're upgrading to 1.16.0 now but the issue is still there...
>> >>
>> >> Did I miss something or is the issue not fixed yet?
>> >>
>> >> Thanks,
>> >> Erik
>> >>
>> >>
>> >> On 10/26/2017 12:45 AM, Dan Haywood wrote:
>> >>> Raised https://issues.apache.org/jira/browse/ISIS-1759 for this
>> >>>
>> >>> On Sun, 1 Oct 2017 at 19:52 Dan Haywood <dan@haywood-associates.co.uk
>> >
>> >>> wrote:
>> >>>
>> >>>> Hi Erik,
>> >>>> I'll look into this... This doesn't sound right that this version is
>> >>>> creating these extra calls into the domain code.
>> >>>> Thx,
>> >>>> Dan
>> >>>>
>> >>>> On Wed, 27 Sep 2017, 11:35 Erik de Hair, <e....@pocos.nl> wrote:
>> >>>>
>> >>>>> Hi,
>> >>>>>
>> >>>>> I'm not really sure about this but it feels like our app's
>> performance
>> >>>>> (Apache Isis 1.15.0) is worse than before and I believe it might be
>> >>>>> because of the fact that the getter and hide method of a property
>> are
>> >>>>> called both, despite the fact that the property is hidden.
>> >>>>>
>> >>>>> We have a (derived) property (or referenced property) defined like
>> >> this:
>> >>>>> public String getX(){
>> >>>>>        return someService.createX(getY(), getZ()); // We don't want
>> this
>> >>>>> method to be called always, because it's an expensive one
>> >>>>> }
>> >>>>> public boolean hideX(){
>> >>>>>        boolean condition = ... // some condition
>> >>>>>        return condition;
>> >>>>> }
>> >>>>>
>> >>>>> In 1.14.0 getX() wouldn't be called if the hideX()-method returned
>> true
>> >>>>> and so wouldn't the expensive service-method be called. For 1.15.0
>> we
>> >>>>> would have to change the implementation to something like the
>> following
>> >>>>> to make sure the expensive call wouldn't be executed:
>> >>>>>
>> >>>>> public String getX(){
>> >>>>>        if(!hideConditionsForX){
>> >>>>>            return someService.createX(getY(), getZ());
>> >>>>>        }
>> >>>>>        return null;
>> >>>>> }
>> >>>>> public boolean hideX(){
>> >>>>>        return hideConditionsForX();
>> >>>>> }
>> >>>>> private boolean hideConditionsForX(){
>> >>>>>        // ... must probably be implemented using QueryResultsCache
>> to
>> >>>>> reduce DB-calls
>> >>>>> }
>> >>>>>
>> >>>>> This is also the case for referenced properties for which no
>> DB-query
>> >>>>> has to be executed if it was hidden. This could count up if we have
>> a
>> >>>>> lot of hidden properties (this might be solved with refactoring).
>> >>>>>
>> >>>>> Am I seeing this correctly?
>> >>>>>
>> >>>>> Erik
>> >>>>>
>> >>>>>
>> >>>>>
>> >>
>>
>>

Re: Always calling getXxx() and hideXxx() both decreases performance of application (?)

Posted by Dan Haywood <da...@haywood-associates.co.uk>.
Hi Erik,

Yes, I'm not sure myself what the impact would be.

I think that the parent EntityModel shouldnt' really have this
responsibility of telling each of its child ScalarModel's to reset
themselves.  Rather, perhaps it should simply instruct the ScalarModels
that they are now dirty, so that if and when the ScalarModels are next
asked for their state (for re-rendering) they then will query the
underlying domain object ... but not before.

I think I'd rather do this in a separate ticket, though, which I'll raise.

thx
Dan


On Fri, 16 Feb 2018 at 10:18 Erik de Hair <e....@pocos.nl> wrote:

> Hi Dan,
>
> We have the same issues on [1] when executing an action.
>
> I can create a pull request and add equal code as for the fix on [2] but
> I can't foresee what a possible impact might be.
>
> Thanks,
> Erik
>
> [1]
>
> https://github.com/apache/isis/blob/master/core/viewer-wicket-model/src/main/java/org/apache/isis/viewer/wicket/model/models/EntityModel.java#L400
> [2]
>
> https://github.com/apache/isis/blob/master/core/viewer-wicket-model/src/main/java/org/apache/isis/viewer/wicket/model/models/ScalarModel.java#L290
>
> On 02/12/2018 04:01 PM, Dan Haywood wrote:
> > Hi Erik,
> >
> > Mistake on my part.  I've brought the ticket back for fix in 1.16.1, and
> > added a comment to the ticket itself.
> >
> > thx
> > Dan
> >
> >
> > On Mon, 12 Feb 2018 at 11:48 Erik de Hair <e....@pocos.nl> wrote:
> >
> >> Hi Dan,
> >>
> >> As far as I can see the fix [1] for ISIS-1759 [2] is not merged to
> >> current release of Apache Isis [3]
> >>
> >> [1]
> https://gitbox.apache.org/repos/asf?p=isis.git;a=commitdiff;h=c6c2fc7
> >> [2] https://issues.apache.org/jira/browse/ISIS-1759
> >> [3]
> >>
> >>
> https://github.com/apache/isis/blob/master/core/viewer-wicket-model/src/main/java/org/apache/isis/viewer/wicket/model/models/ScalarModel.java#L290
> >>
> >> We're upgrading to 1.16.0 now but the issue is still there...
> >>
> >> Did I miss something or is the issue not fixed yet?
> >>
> >> Thanks,
> >> Erik
> >>
> >>
> >> On 10/26/2017 12:45 AM, Dan Haywood wrote:
> >>> Raised https://issues.apache.org/jira/browse/ISIS-1759 for this
> >>>
> >>> On Sun, 1 Oct 2017 at 19:52 Dan Haywood <da...@haywood-associates.co.uk>
> >>> wrote:
> >>>
> >>>> Hi Erik,
> >>>> I'll look into this... This doesn't sound right that this version is
> >>>> creating these extra calls into the domain code.
> >>>> Thx,
> >>>> Dan
> >>>>
> >>>> On Wed, 27 Sep 2017, 11:35 Erik de Hair, <e....@pocos.nl> wrote:
> >>>>
> >>>>> Hi,
> >>>>>
> >>>>> I'm not really sure about this but it feels like our app's
> performance
> >>>>> (Apache Isis 1.15.0) is worse than before and I believe it might be
> >>>>> because of the fact that the getter and hide method of a property are
> >>>>> called both, despite the fact that the property is hidden.
> >>>>>
> >>>>> We have a (derived) property (or referenced property) defined like
> >> this:
> >>>>> public String getX(){
> >>>>>        return someService.createX(getY(), getZ()); // We don't want
> this
> >>>>> method to be called always, because it's an expensive one
> >>>>> }
> >>>>> public boolean hideX(){
> >>>>>        boolean condition = ... // some condition
> >>>>>        return condition;
> >>>>> }
> >>>>>
> >>>>> In 1.14.0 getX() wouldn't be called if the hideX()-method returned
> true
> >>>>> and so wouldn't the expensive service-method be called. For 1.15.0 we
> >>>>> would have to change the implementation to something like the
> following
> >>>>> to make sure the expensive call wouldn't be executed:
> >>>>>
> >>>>> public String getX(){
> >>>>>        if(!hideConditionsForX){
> >>>>>            return someService.createX(getY(), getZ());
> >>>>>        }
> >>>>>        return null;
> >>>>> }
> >>>>> public boolean hideX(){
> >>>>>        return hideConditionsForX();
> >>>>> }
> >>>>> private boolean hideConditionsForX(){
> >>>>>        // ... must probably be implemented using QueryResultsCache to
> >>>>> reduce DB-calls
> >>>>> }
> >>>>>
> >>>>> This is also the case for referenced properties for which no DB-query
> >>>>> has to be executed if it was hidden. This could count up if we have a
> >>>>> lot of hidden properties (this might be solved with refactoring).
> >>>>>
> >>>>> Am I seeing this correctly?
> >>>>>
> >>>>> Erik
> >>>>>
> >>>>>
> >>>>>
> >>
>
>

Re: Always calling getXxx() and hideXxx() both decreases performance of application (?)

Posted by Erik de Hair <e....@pocos.nl>.
Hi Dan,

We have the same issues on [1] when executing an action.

I can create a pull request and add equal code as for the fix on [2] but 
I can't foresee what a possible impact might be.

Thanks,
Erik

[1] 
https://github.com/apache/isis/blob/master/core/viewer-wicket-model/src/main/java/org/apache/isis/viewer/wicket/model/models/EntityModel.java#L400
[2] 
https://github.com/apache/isis/blob/master/core/viewer-wicket-model/src/main/java/org/apache/isis/viewer/wicket/model/models/ScalarModel.java#L290

On 02/12/2018 04:01 PM, Dan Haywood wrote:
> Hi Erik,
>
> Mistake on my part.  I've brought the ticket back for fix in 1.16.1, and
> added a comment to the ticket itself.
>
> thx
> Dan
>
>
> On Mon, 12 Feb 2018 at 11:48 Erik de Hair <e....@pocos.nl> wrote:
>
>> Hi Dan,
>>
>> As far as I can see the fix [1] for ISIS-1759 [2] is not merged to
>> current release of Apache Isis [3]
>>
>> [1] https://gitbox.apache.org/repos/asf?p=isis.git;a=commitdiff;h=c6c2fc7
>> [2] https://issues.apache.org/jira/browse/ISIS-1759
>> [3]
>>
>> https://github.com/apache/isis/blob/master/core/viewer-wicket-model/src/main/java/org/apache/isis/viewer/wicket/model/models/ScalarModel.java#L290
>>
>> We're upgrading to 1.16.0 now but the issue is still there...
>>
>> Did I miss something or is the issue not fixed yet?
>>
>> Thanks,
>> Erik
>>
>>
>> On 10/26/2017 12:45 AM, Dan Haywood wrote:
>>> Raised https://issues.apache.org/jira/browse/ISIS-1759 for this
>>>
>>> On Sun, 1 Oct 2017 at 19:52 Dan Haywood <da...@haywood-associates.co.uk>
>>> wrote:
>>>
>>>> Hi Erik,
>>>> I'll look into this... This doesn't sound right that this version is
>>>> creating these extra calls into the domain code.
>>>> Thx,
>>>> Dan
>>>>
>>>> On Wed, 27 Sep 2017, 11:35 Erik de Hair, <e....@pocos.nl> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I'm not really sure about this but it feels like our app's performance
>>>>> (Apache Isis 1.15.0) is worse than before and I believe it might be
>>>>> because of the fact that the getter and hide method of a property are
>>>>> called both, despite the fact that the property is hidden.
>>>>>
>>>>> We have a (derived) property (or referenced property) defined like
>> this:
>>>>> public String getX(){
>>>>>        return someService.createX(getY(), getZ()); // We don't want this
>>>>> method to be called always, because it's an expensive one
>>>>> }
>>>>> public boolean hideX(){
>>>>>        boolean condition = ... // some condition
>>>>>        return condition;
>>>>> }
>>>>>
>>>>> In 1.14.0 getX() wouldn't be called if the hideX()-method returned true
>>>>> and so wouldn't the expensive service-method be called. For 1.15.0 we
>>>>> would have to change the implementation to something like the following
>>>>> to make sure the expensive call wouldn't be executed:
>>>>>
>>>>> public String getX(){
>>>>>        if(!hideConditionsForX){
>>>>>            return someService.createX(getY(), getZ());
>>>>>        }
>>>>>        return null;
>>>>> }
>>>>> public boolean hideX(){
>>>>>        return hideConditionsForX();
>>>>> }
>>>>> private boolean hideConditionsForX(){
>>>>>        // ... must probably be implemented using QueryResultsCache to
>>>>> reduce DB-calls
>>>>> }
>>>>>
>>>>> This is also the case for referenced properties for which no DB-query
>>>>> has to be executed if it was hidden. This could count up if we have a
>>>>> lot of hidden properties (this might be solved with refactoring).
>>>>>
>>>>> Am I seeing this correctly?
>>>>>
>>>>> Erik
>>>>>
>>>>>
>>>>>
>>


Re: Always calling getXxx() and hideXxx() both decreases performance of application (?)

Posted by Dan Haywood <da...@haywood-associates.co.uk>.
Hi Erik,

Mistake on my part.  I've brought the ticket back for fix in 1.16.1, and
added a comment to the ticket itself.

thx
Dan


On Mon, 12 Feb 2018 at 11:48 Erik de Hair <e....@pocos.nl> wrote:

> Hi Dan,
>
> As far as I can see the fix [1] for ISIS-1759 [2] is not merged to
> current release of Apache Isis [3]
>
> [1] https://gitbox.apache.org/repos/asf?p=isis.git;a=commitdiff;h=c6c2fc7
> [2] https://issues.apache.org/jira/browse/ISIS-1759
> [3]
>
> https://github.com/apache/isis/blob/master/core/viewer-wicket-model/src/main/java/org/apache/isis/viewer/wicket/model/models/ScalarModel.java#L290
>
> We're upgrading to 1.16.0 now but the issue is still there...
>
> Did I miss something or is the issue not fixed yet?
>
> Thanks,
> Erik
>
>
> On 10/26/2017 12:45 AM, Dan Haywood wrote:
> > Raised https://issues.apache.org/jira/browse/ISIS-1759 for this
> >
> > On Sun, 1 Oct 2017 at 19:52 Dan Haywood <da...@haywood-associates.co.uk>
> > wrote:
> >
> >> Hi Erik,
> >> I'll look into this... This doesn't sound right that this version is
> >> creating these extra calls into the domain code.
> >> Thx,
> >> Dan
> >>
> >> On Wed, 27 Sep 2017, 11:35 Erik de Hair, <e....@pocos.nl> wrote:
> >>
> >>> Hi,
> >>>
> >>> I'm not really sure about this but it feels like our app's performance
> >>> (Apache Isis 1.15.0) is worse than before and I believe it might be
> >>> because of the fact that the getter and hide method of a property are
> >>> called both, despite the fact that the property is hidden.
> >>>
> >>> We have a (derived) property (or referenced property) defined like
> this:
> >>>
> >>> public String getX(){
> >>>       return someService.createX(getY(), getZ()); // We don't want this
> >>> method to be called always, because it's an expensive one
> >>> }
> >>> public boolean hideX(){
> >>>       boolean condition = ... // some condition
> >>>       return condition;
> >>> }
> >>>
> >>> In 1.14.0 getX() wouldn't be called if the hideX()-method returned true
> >>> and so wouldn't the expensive service-method be called. For 1.15.0 we
> >>> would have to change the implementation to something like the following
> >>> to make sure the expensive call wouldn't be executed:
> >>>
> >>> public String getX(){
> >>>       if(!hideConditionsForX){
> >>>           return someService.createX(getY(), getZ());
> >>>       }
> >>>       return null;
> >>> }
> >>> public boolean hideX(){
> >>>       return hideConditionsForX();
> >>> }
> >>> private boolean hideConditionsForX(){
> >>>       // ... must probably be implemented using QueryResultsCache to
> >>> reduce DB-calls
> >>> }
> >>>
> >>> This is also the case for referenced properties for which no DB-query
> >>> has to be executed if it was hidden. This could count up if we have a
> >>> lot of hidden properties (this might be solved with refactoring).
> >>>
> >>> Am I seeing this correctly?
> >>>
> >>> Erik
> >>>
> >>>
> >>>
>
>