You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gilles Sadowski <gi...@harfang.homelinux.org> on 2012/12/15 22:45:50 UTC

Re: svn commit: r1422321 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java

On Sat, Dec 15, 2012 at 07:24:05PM -0000, psteitz@apache.org wrote:
> Author: psteitz
> Date: Sat Dec 15 19:24:04 2012
> New Revision: 1422321
> 
> URL: http://svn.apache.org/viewvc?rev=1422321&view=rev
> Log:
> Added sync to compute method.
> 
> Modified:
>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java
> 
> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java
> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java?rev=1422321&r1=1422320&r2=1422321&view=diff
> ==============================================================================
> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java (original)
> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java Sat Dec 15 19:24:04 2012
> @@ -930,7 +930,7 @@ public class ResizableDoubleArray implem
>       * @return the result.
>       * @since 3.1
>       */
> -    public double compute(MathArrays.Function f) {
> +    public synchronized double compute(MathArrays.Function f) {
>          return f.evaluate(internalArray, startIndex, numElements);
>      }

Didn't we conclude that all "synchronized" keywords should be dropped?


Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1422321 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java

Posted by Phil Steitz <ph...@gmail.com>.
On 12/15/12 4:28 PM, Gilles Sadowski wrote:
> On Sat, Dec 15, 2012 at 02:15:20PM -0800, Phil Steitz wrote:
>> On 12/15/12 1:45 PM, Gilles Sadowski wrote:
>>> On Sat, Dec 15, 2012 at 07:24:05PM -0000, psteitz@apache.org wrote:
>>>> Author: psteitz
>>>> Date: Sat Dec 15 19:24:04 2012
>>>> New Revision: 1422321
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1422321&view=rev
>>>> Log:
>>>> Added sync to compute method.
>>>>
>>>> Modified:
>>>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java
>>>>
>>>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java
>>>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java?rev=1422321&r1=1422320&r2=1422321&view=diff
>>>> ==============================================================================
>>>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java (original)
>>>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java Sat Dec 15 19:24:04 2012
>>>> @@ -930,7 +930,7 @@ public class ResizableDoubleArray implem
>>>>       * @return the result.
>>>>       * @since 3.1
>>>>       */
>>>> -    public double compute(MathArrays.Function f) {
>>>> +    public synchronized double compute(MathArrays.Function f) {
>>>>          return f.evaluate(internalArray, startIndex, numElements);
>>>>      }
>>> Didn't we conclude that all "synchronized" keywords should be dropped?
>> If that is the case, we should remove them uniformly from this
>> class.  As it is now, the class is thread-safe.
> If that's so, why isn't MATH-757 resolved?
>
>>  Above was the only
>> method accessing member data that was not synchronized.
> Others remain.
>
>> I think we concluded that we could not remove the sync protection
>> that was there in a dot release.
> Yes. We agreed not to remove "synchronized" where it existed.
>
>>  The above method is new and breaks
>> threadsafety unless protected.
> Thread-safety was broken _before_. We agreed that it was better not to try
> and fix it (as this is a low-level class, it is obviously more flexible to
> let users handle synchronization if they need it).
>
> The goal was to (hopefully) ensure that legacy code (user application) will
> have their behaviour unchanged by this release _provided they do not change
> their code_ (i.e. not use the new features).
> It was certainly not to encourage new supposedly thread-safe uses of that
> class when we know that the (supposed) thread-safety support will be dropped
> in 4.0.  This would be evil. :-)
>
> In summary, the new methods should not be "synchronized", if just to draw
> the attention of careful users that their legacy code may suffer from
> potential problems.
> I propose to revert that change, and add a comment in the class
> documentation about the future removal of all "synchronized" keywords.

OK

Phil
>
>
> Gilles
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1422321 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Sat, Dec 15, 2012 at 02:15:20PM -0800, Phil Steitz wrote:
> On 12/15/12 1:45 PM, Gilles Sadowski wrote:
> > On Sat, Dec 15, 2012 at 07:24:05PM -0000, psteitz@apache.org wrote:
> >> Author: psteitz
> >> Date: Sat Dec 15 19:24:04 2012
> >> New Revision: 1422321
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1422321&view=rev
> >> Log:
> >> Added sync to compute method.
> >>
> >> Modified:
> >>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java
> >>
> >> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java
> >> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java?rev=1422321&r1=1422320&r2=1422321&view=diff
> >> ==============================================================================
> >> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java (original)
> >> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java Sat Dec 15 19:24:04 2012
> >> @@ -930,7 +930,7 @@ public class ResizableDoubleArray implem
> >>       * @return the result.
> >>       * @since 3.1
> >>       */
> >> -    public double compute(MathArrays.Function f) {
> >> +    public synchronized double compute(MathArrays.Function f) {
> >>          return f.evaluate(internalArray, startIndex, numElements);
> >>      }
> > Didn't we conclude that all "synchronized" keywords should be dropped?
> 
> If that is the case, we should remove them uniformly from this
> class.  As it is now, the class is thread-safe.

If that's so, why isn't MATH-757 resolved?

>  Above was the only
> method accessing member data that was not synchronized.

Others remain.

> I think we concluded that we could not remove the sync protection
> that was there in a dot release.

Yes. We agreed not to remove "synchronized" where it existed.

>  The above method is new and breaks
> threadsafety unless protected.

Thread-safety was broken _before_. We agreed that it was better not to try
and fix it (as this is a low-level class, it is obviously more flexible to
let users handle synchronization if they need it).

The goal was to (hopefully) ensure that legacy code (user application) will
have their behaviour unchanged by this release _provided they do not change
their code_ (i.e. not use the new features).
It was certainly not to encourage new supposedly thread-safe uses of that
class when we know that the (supposed) thread-safety support will be dropped
in 4.0.  This would be evil. :-)

In summary, the new methods should not be "synchronized", if just to draw
the attention of careful users that their legacy code may suffer from
potential problems.
I propose to revert that change, and add a comment in the class
documentation about the future removal of all "synchronized" keywords.


Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1422321 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java

Posted by Phil Steitz <ph...@gmail.com>.
On 12/15/12 1:45 PM, Gilles Sadowski wrote:
> On Sat, Dec 15, 2012 at 07:24:05PM -0000, psteitz@apache.org wrote:
>> Author: psteitz
>> Date: Sat Dec 15 19:24:04 2012
>> New Revision: 1422321
>>
>> URL: http://svn.apache.org/viewvc?rev=1422321&view=rev
>> Log:
>> Added sync to compute method.
>>
>> Modified:
>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java
>>
>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java?rev=1422321&r1=1422320&r2=1422321&view=diff
>> ==============================================================================
>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java (original)
>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/ResizableDoubleArray.java Sat Dec 15 19:24:04 2012
>> @@ -930,7 +930,7 @@ public class ResizableDoubleArray implem
>>       * @return the result.
>>       * @since 3.1
>>       */
>> -    public double compute(MathArrays.Function f) {
>> +    public synchronized double compute(MathArrays.Function f) {
>>          return f.evaluate(internalArray, startIndex, numElements);
>>      }
> Didn't we conclude that all "synchronized" keywords should be dropped?

If that is the case, we should remove them uniformly from this
class.  As it is now, the class is thread-safe.  Above was the only
method accessing member data that was not synchronized.
I think we concluded that we could not remove the sync protection
that was there in a dot release.  The above method is new and breaks
threadsafety unless protected.

Phil
>
>
> Gilles
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org