GenericValue.getRelatedOne/Cache

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

GenericValue.getRelatedOne/Cache

Adam Heath-2
A while back, I started adding more variants of
GenericDelegator.findByPrimaryKey.  The outcome of that was to remove
those variants, and reduce the methods.

However, while looking at unrelated code tonight, I thought we should
do the same to the lookup methods in GenericValue.  For instance, I
saw this pattern:

if (booleanValue) {
  nextValue = value.getRelatedOneCache(relation);
} else {
  nextValue = value.getRelatedOne(relation);
}

I think it would be better to change that to getRelatedOne(relation,
boolean).

Do others agree?  What about the other methods in that class?
Reply | Threaded
Open this post in threaded view
|

Re: GenericValue.getRelatedOne/Cache

Jacques Le Roux
Administrator
+1

Jacques

From: "Adam Heath" <[hidden email]>

>A while back, I started adding more variants of
> GenericDelegator.findByPrimaryKey.  The outcome of that was to remove
> those variants, and reduce the methods.
>
> However, while looking at unrelated code tonight, I thought we should
> do the same to the lookup methods in GenericValue.  For instance, I
> saw this pattern:
>
> if (booleanValue) {
>  nextValue = value.getRelatedOneCache(relation);
> } else {
>  nextValue = value.getRelatedOne(relation);
> }
>
> I think it would be better to change that to getRelatedOne(relation,
> boolean).
>
> Do others agree?  What about the other methods in that class?
>

Reply | Threaded
Open this post in threaded view
|

Re: GenericValue.getRelatedOne/Cache

Sascha Rodekamp-3
In reply to this post by Adam Heath-2
Good point Adam.
+1

Am 03.12.2010 um 07:39 schrieb Adam Heath <[hidden email]>:

> A while back, I started adding more variants of
> GenericDelegator.findByPrimaryKey.  The outcome of that was to remove
> those variants, and reduce the methods.
>
> However, while looking at unrelated code tonight, I thought we should
> do the same to the lookup methods in GenericValue.  For instance, I
> saw this pattern:
>
> if (booleanValue) {
>  nextValue = value.getRelatedOneCache(relation);
> } else {
>  nextValue = value.getRelatedOne(relation);
> }
>
> I think it would be better to change that to getRelatedOne(relation,
> boolean).
>
> Do others agree?  What about the other methods in that class?
Reply | Threaded
Open this post in threaded view
|

Re: GenericValue.getRelatedOne/Cache

Marc Morin
In reply to this post by Adam Heath-2
Just my $0.02, but I am not a fan of the cache/no-cache paradigm in ofbiz.  Forcing the application developer to know about the cache leads to unstable code and usually slower execution of the application (ie. being conservative and saying cache-off, since it MAY be modified).

The cache is something that the application developer should never need to concern themselves with.  Of course, with ofbiz, this isn't the case, and it has to do with the fact that the cache returns immutable objects.

I'd like to open up a discussion about changing/improving the implementation of the entity/condition cache layer to improve this.  The goals are:

- cache is passively managed by the framework. Application layer NEVER exposes cache boolean or cache variant methods.
- goal to maintain a single object reference for the same pkey.  (ie.  findByPrimaryKey() and and search by and returning same value point to the same instance).
- soft reference on cache (ofbiz does this already).
- weak reference on entities marked "non-cached" (if it's in the jvm memory already, why not return it).
- entity definition cache flag, as it is now. (controls hard/soft vs weak)
- respect transaction boundaries (current cache doesn't... try insert entity, find it, rollback.... entity remains in cache).
- objects fetched from delegator are always mutable.  (use a copy on write semantic for cache).
- distributed cache semantics (already in ofbiz)
- nested views on views and proper cache behavior

Now, I am not just trying to create a make work project here.   We have already implemented all of these changes in our application's use of ofbiz.  I'd be prepared to package this up and contribute it back to the community.   Please advise.


Marc Morin
Emforium Group Inc.
ALL-IN Software
519-772-6824 ext 201
[hidden email]

----- Original Message -----

> A while back, I started adding more variants of
> GenericDelegator.findByPrimaryKey. The outcome of that was to remove
> those variants, and reduce the methods.
>
> However, while looking at unrelated code tonight, I thought we should
> do the same to the lookup methods in GenericValue. For instance, I
> saw this pattern:
>
> if (booleanValue) {
> nextValue = value.getRelatedOneCache(relation);
> } else {
> nextValue = value.getRelatedOne(relation);
> }
>
> I think it would be better to change that to getRelatedOne(relation,
> boolean).
>
> Do others agree? What about the other methods in that class?
Reply | Threaded
Open this post in threaded view
|

Re: GenericValue.getRelatedOne/Cache

Adrian Crum
In reply to this post by Adam Heath-2
+1

-Adrian

On 12/2/2010 10:39 PM, Adam Heath wrote:

> A while back, I started adding more variants of
> GenericDelegator.findByPrimaryKey.  The outcome of that was to remove
> those variants, and reduce the methods.
>
> However, while looking at unrelated code tonight, I thought we should
> do the same to the lookup methods in GenericValue.  For instance, I
> saw this pattern:
>
> if (booleanValue) {
>    nextValue = value.getRelatedOneCache(relation);
> } else {
>    nextValue = value.getRelatedOne(relation);
> }
>
> I think it would be better to change that to getRelatedOne(relation,
> boolean).
>
> Do others agree?  What about the other methods in that class?
>
Reply | Threaded
Open this post in threaded view
|

Re: GenericValue.getRelatedOne/Cache

Jacques Le Roux
Administrator
I'm quite sure Adrian and I will not be the only ones interested...

Thanks

Jacques

From: "Adrian Crum" <[hidden email]>

> +1
>
> -Adrian
>
> On 12/2/2010 10:39 PM, Adam Heath wrote:
>> A while back, I started adding more variants of
>> GenericDelegator.findByPrimaryKey.  The outcome of that was to remove
>> those variants, and reduce the methods.
>>
>> However, while looking at unrelated code tonight, I thought we should
>> do the same to the lookup methods in GenericValue.  For instance, I
>> saw this pattern:
>>
>> if (booleanValue) {
>>    nextValue = value.getRelatedOneCache(relation);
>> } else {
>>    nextValue = value.getRelatedOne(relation);
>> }
>>
>> I think it would be better to change that to getRelatedOne(relation,
>> boolean).
>>
>> Do others agree?  What about the other methods in that class?
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: GenericValue.getRelatedOne/Cache

Adam Heath-2
Jacques Le Roux wrote:
> I'm quite sure Adrian and I will not be the only ones interested...

Figured as much.  I just started working on it.