OFBiz › OFBiz - Dev

Re: svn commit: r956472 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java

‹ Previous Topic Next Topic ›
 
classic Classic list List threaded Threaded
♦
♦
 6 messages Options
Scott Gray-2
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: svn commit: r956472 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java

Scott Gray-2
Won't that result in the bundle being loaded twice if another thread is waiting for entry to create the same bundle?

Regards
Scott

On 21/06/2010, at 7:09 PM, [hidden email] wrote:

> Author: doogie
> Date: Mon Jun 21 07:09:13 2010
> New Revision: 956472
>
> URL: http://svn.apache.org/viewvc?rev=956472&view=rev
> Log:
> Remove useless check for non-null bundle, immediately after a null
> check, in UtilResourceBundle.getBundle().
>
> Modified:
>    ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
>
> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java?rev=956472&r1=956471&r2=956472&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java Mon Jun 21 07:09:13 2010
> @@ -896,9 +896,6 @@ public class UtilProperties implements S
>             UtilResourceBundle bundle = bundleCache.get(resourceName);
>             if (bundle == null) {
>                 synchronized (bundleCache) {
> -                    if (bundle != null) {
> -                        return bundle;
> -                    }
>                     double startTime = System.currentTimeMillis();
>                     FastList<Locale> candidateLocales = (FastList<Locale>) getCandidateLocales(locale);
>                     UtilResourceBundle parentBundle = null;
>
>


smime.p7s (3K) Download Attachment
Adam Heath-2
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: svn commit: r956472 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java

Adam Heath-2
Scott Gray wrote:
> Won't that result in the bundle being loaded twice if another thread is waiting for entry to create the same bundle?

no.  read it again.  the bundle is not reassigned between the first
null check, and the second not null check.  there is no way for the
second if block to ever actually do anything.

>
> Regards
> Scott
>
> On 21/06/2010, at 7:09 PM, [hidden email] wrote:
>
>> Author: doogie
>> Date: Mon Jun 21 07:09:13 2010
>> New Revision: 956472
>>
>> URL: http://svn.apache.org/viewvc?rev=956472&view=rev
>> Log:
>> Remove useless check for non-null bundle, immediately after a null
>> check, in UtilResourceBundle.getBundle().
>>
>> Modified:
>>    ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
>>
>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java?rev=956472&r1=956471&r2=956472&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java (original)
>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java Mon Jun 21 07:09:13 2010
>> @@ -896,9 +896,6 @@ public class UtilProperties implements S
>>             UtilResourceBundle bundle = bundleCache.get(resourceName);
>>             if (bundle == null) {
>>                 synchronized (bundleCache) {
>> -                    if (bundle != null) {
>> -                        return bundle;
>> -                    }
>>                     double startTime = System.currentTimeMillis();
>>                     FastList<Locale> candidateLocales = (FastList<Locale>) getCandidateLocales(locale);
>>                     UtilResourceBundle parentBundle = null;
>>
>>
>

Scott Gray-2
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: svn commit: r956472 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java

Scott Gray-2
On 21/06/2010, at 7:26 PM, Adam Heath wrote:

> Scott Gray wrote:
>> Won't that result in the bundle being loaded twice if another thread is waiting for entry to create the same bundle?
>
> no.  read it again.  the bundle is not reassigned between the first
> null check, and the second not null check.  there is no way for the
> second if block to ever actually do anything.

Gotcha, the double loading still applies though, why not recheck the cache before proceeding?

>>
>> Regards
>> Scott
>>
>> On 21/06/2010, at 7:09 PM, [hidden email] wrote:
>>
>>> Author: doogie
>>> Date: Mon Jun 21 07:09:13 2010
>>> New Revision: 956472
>>>
>>> URL: http://svn.apache.org/viewvc?rev=956472&view=rev
>>> Log:
>>> Remove useless check for non-null bundle, immediately after a null
>>> check, in UtilResourceBundle.getBundle().
>>>
>>> Modified:
>>>   ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
>>>
>>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java?rev=956472&r1=956471&r2=956472&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java (original)
>>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java Mon Jun 21 07:09:13 2010
>>> @@ -896,9 +896,6 @@ public class UtilProperties implements S
>>>            UtilResourceBundle bundle = bundleCache.get(resourceName);
>>>            if (bundle == null) {
>>>                synchronized (bundleCache) {
>>> -                    if (bundle != null) {
>>> -                        return bundle;
>>> -                    }
>>>                    double startTime = System.currentTimeMillis();
>>>                    FastList<Locale> candidateLocales = (FastList<Locale>) getCandidateLocales(locale);
>>>                    UtilResourceBundle parentBundle = null;
>>>
>>>
>>
>


smime.p7s (3K) Download Attachment
Adam Heath-2
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: svn commit: r956472 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java

Adam Heath-2
Scott Gray wrote:
> On 21/06/2010, at 7:26 PM, Adam Heath wrote:
>
>> Scott Gray wrote:
>>> Won't that result in the bundle being loaded twice if another thread is waiting for entry to create the same bundle?
>> no.  read it again.  the bundle is not reassigned between the first
>> null check, and the second not null check.  there is no way for the
>> second if block to ever actually do anything.
>
> Gotcha, the double loading still applies though, why not recheck the cache before proceeding?

Because a later patch removes the synchronized completely.  still
working on that and others tho.

>
>>> Regards
>>> Scott
>>>
>>> On 21/06/2010, at 7:09 PM, [hidden email] wrote:
>>>
>>>> Author: doogie
>>>> Date: Mon Jun 21 07:09:13 2010
>>>> New Revision: 956472
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=956472&view=rev
>>>> Log:
>>>> Remove useless check for non-null bundle, immediately after a null
>>>> check, in UtilResourceBundle.getBundle().
>>>>
>>>> Modified:
>>>>   ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
>>>>
>>>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java?rev=956472&r1=956471&r2=956472&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java (original)
>>>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java Mon Jun 21 07:09:13 2010
>>>> @@ -896,9 +896,6 @@ public class UtilProperties implements S
>>>>            UtilResourceBundle bundle = bundleCache.get(resourceName);
>>>>            if (bundle == null) {
>>>>                synchronized (bundleCache) {
>>>> -                    if (bundle != null) {
>>>> -                        return bundle;
>>>> -                    }
>>>>                    double startTime = System.currentTimeMillis();
>>>>                    FastList<Locale> candidateLocales = (FastList<Locale>) getCandidateLocales(locale);
>>>>                    UtilResourceBundle parentBundle = null;
>>>>
>>>>
>

Adam Heath-2
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: svn commit: r956472 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java

Adam Heath-2
In reply to this post by Scott Gray-2
Scott Gray wrote:
> On 21/06/2010, at 7:26 PM, Adam Heath wrote:
>
>> Scott Gray wrote:
>>> Won't that result in the bundle being loaded twice if another thread is waiting for entry to create the same bundle?
>> no.  read it again.  the bundle is not reassigned between the first
>> null check, and the second not null check.  there is no way for the
>> second if block to ever actually do anything.
>
> Gotcha, the double loading still applies though, why not recheck the cache before proceeding?

My change has nothing to do with double loading, that problem was
already there previously.  I just removed purely useless code.


>
>>> Regards
>>> Scott
>>>
>>> On 21/06/2010, at 7:09 PM, [hidden email] wrote:
>>>
>>>> Author: doogie
>>>> Date: Mon Jun 21 07:09:13 2010
>>>> New Revision: 956472
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=956472&view=rev
>>>> Log:
>>>> Remove useless check for non-null bundle, immediately after a null
>>>> check, in UtilResourceBundle.getBundle().
>>>>
>>>> Modified:
>>>>   ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
>>>>
>>>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java?rev=956472&r1=956471&r2=956472&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java (original)
>>>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java Mon Jun 21 07:09:13 2010
>>>> @@ -896,9 +896,6 @@ public class UtilProperties implements S
>>>>            UtilResourceBundle bundle = bundleCache.get(resourceName);
>>>>            if (bundle == null) {
>>>>                synchronized (bundleCache) {
>>>> -                    if (bundle != null) {
>>>> -                        return bundle;
>>>> -                    }
>>>>                    double startTime = System.currentTimeMillis();
>>>>                    FastList<Locale> candidateLocales = (FastList<Locale>) getCandidateLocales(locale);
>>>>                    UtilResourceBundle parentBundle = null;
>>>>
>>>>
>

Scott Gray-2
Reply | Threaded
Open this post in threaded view
♦
♦
|

Re: svn commit: r956472 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java

Scott Gray-2
In reply to this post by Adam Heath-2
On 21/06/2010, at 7:49 PM, Adam Heath wrote:

> Scott Gray wrote:
>> On 21/06/2010, at 7:26 PM, Adam Heath wrote:
>>
>>> Scott Gray wrote:
>>>> Won't that result in the bundle being loaded twice if another thread is waiting for entry to create the same bundle?
>>> no.  read it again.  the bundle is not reassigned between the first
>>> null check, and the second not null check.  there is no way for the
>>> second if block to ever actually do anything.
>>
>> Gotcha, the double loading still applies though, why not recheck the cache before proceeding?
>
> Because a later patch removes the synchronized completely.  still
> working on that and others tho.
Sounds good to me.

>>
>>>> Regards
>>>> Scott
>>>>
>>>> On 21/06/2010, at 7:09 PM, [hidden email] wrote:
>>>>
>>>>> Author: doogie
>>>>> Date: Mon Jun 21 07:09:13 2010
>>>>> New Revision: 956472
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=956472&view=rev
>>>>> Log:
>>>>> Remove useless check for non-null bundle, immediately after a null
>>>>> check, in UtilResourceBundle.getBundle().
>>>>>
>>>>> Modified:
>>>>>  ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
>>>>>
>>>>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java?rev=956472&r1=956471&r2=956472&view=diff
>>>>> ==============================================================================
>>>>> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java (original)
>>>>> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java Mon Jun 21 07:09:13 2010
>>>>> @@ -896,9 +896,6 @@ public class UtilProperties implements S
>>>>>           UtilResourceBundle bundle = bundleCache.get(resourceName);
>>>>>           if (bundle == null) {
>>>>>               synchronized (bundleCache) {
>>>>> -                    if (bundle != null) {
>>>>> -                        return bundle;
>>>>> -                    }
>>>>>                   double startTime = System.currentTimeMillis();
>>>>>                   FastList<Locale> candidateLocales = (FastList<Locale>) getCandidateLocales(locale);
>>>>>                   UtilResourceBundle parentBundle = null;
>>>>>
>>>>>
>>
>


smime.p7s (3K) Download Attachment
Free forum by Nabble Edit this page