Re: RFR 8029452: Fork/Join task ForEachOps.ForEachOrderedTask clarifications and minor improvements

2014-01-16 Thread Paul Sandoz

On Jan 10, 2014, at 2:42 PM, Paul Sandoz paul.san...@oracle.com wrote:
 I have also removed the inconsistently applied synchronized block. Either we 
 apply it consistently to reporting or not at all. It was originally there 
 because we were not sure that the happens-before relationship [1] between 
 elements would be guaranteed. However, ForEachOrderedTask sets up such a 
 relationship via completion counts to ensure leaf nodes complete in encounter 
 order (if any) where only one leaf can be completing (which was left most 
 leaf that was not completed), hence stamping a fence in the ground at these 
 point seems redundant (at least i cannot see its value but could be missing 
 something subtle).
 

I updated with some more comments explaining how the happens-before is 
preserved:

  http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8029452-ForEachOrdered/webrev/

Paul.


Re: RFR 8029452: Fork/Join task ForEachOps.ForEachOrderedTask clarifications and minor improvements

2014-01-16 Thread Mike Duigou
Very helpful. Thank you for adding the comments.

Mike

On Jan 16 2014, at 03:26 , Paul Sandoz paul.san...@oracle.com wrote:

 
 On Jan 10, 2014, at 2:42 PM, Paul Sandoz paul.san...@oracle.com wrote:
 I have also removed the inconsistently applied synchronized block. Either we 
 apply it consistently to reporting or not at all. It was originally there 
 because we were not sure that the happens-before relationship [1] between 
 elements would be guaranteed. However, ForEachOrderedTask sets up such a 
 relationship via completion counts to ensure leaf nodes complete in 
 encounter order (if any) where only one leaf can be completing (which was 
 left most leaf that was not completed), hence stamping a fence in the ground 
 at these point seems redundant (at least i cannot see its value but could be 
 missing something subtle).
 
 
 I updated with some more comments explaining how the happens-before is 
 preserved:
 
  http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8029452-ForEachOrdered/webrev/
 
 Paul.



RFR 8029452: Fork/Join task ForEachOps.ForEachOrderedTask clarifications and minor improvements

2014-01-10 Thread Paul Sandoz
Hi,

Some tweaks to the Stream forEachOrdered operation:

  http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8029452-ForEachOrdered/webrev/

The first tweak is to size the CHM used in ForEachOrderedTask, this avoids 
concurrent resizes and the costs associated with those.

The second tweak is to consolidate the reporting of elements to within the 
ForEachOrderedTask.tryComplete method. 

I have also removed the inconsistently applied synchronized block. Either we 
apply it consistently to reporting or not at all. It was originally there 
because we were not sure that the happens-before relationship [1] between 
elements would be guaranteed. However, ForEachOrderedTask sets up such a 
relationship via completion counts to ensure leaf nodes complete in encounter 
order (if any) where only one leaf can be completing (which was left most leaf 
that was not completed), hence stamping a fence in the ground at these point 
seems redundant (at least i cannot see its value but could be missing something 
subtle).

Paul.

[1]
 * pThis operation processes the elements one at a time, in encounter
 * order if one exists.  Performing the action for one element
 * a 
href=../concurrent/package-summary.html#MemoryVisibilityihappens-before/i/a
 * performing the action for subsequent elements, but for any given element,
 * the action may be performed in whatever thread the library chooses.
 *
 * @param action a a href=package-summary.html#NonInterference
 *   non-interfering/a action to perform on the elements
 * @see #forEach(Consumer)
 */
void forEachOrdered(Consumer? super T action);



Re: RFR 8029452: Fork/Join task ForEachOps.ForEachOrderedTask clarifications and minor improvements

2014-01-10 Thread Mike Duigou

On Jan 10 2014, at 05:42 , Paul Sandoz paul.san...@oracle.com wrote:

 Hi,
 
 Some tweaks to the Stream forEachOrdered operation:
 
  http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8029452-ForEachOrdered/webrev/
 
 The first tweak is to size the CHM used in ForEachOrderedTask, this avoids 
 concurrent resizes and the costs associated with those.

+1

 
 The second tweak is to consolidate the reporting of elements to within the 
 ForEachOrderedTask.tryComplete method. 
 
 I have also removed the inconsistently applied synchronized block. Either we 
 apply it consistently to reporting or not at all. It was originally there 
 because we were not sure that the happens-before relationship [1] between 
 elements would be guaranteed. However, ForEachOrderedTask sets up such a 
 relationship via completion counts to ensure leaf nodes complete in encounter 
 order (if any) where only one leaf can be completing (which was left most 
 leaf that was not completed), hence stamping a fence in the ground at these 
 point seems redundant (at least i cannot see its value but could be missing 
 something subtle).

Coud not the lock object be removed?

Mike

 
 Paul.
 
 [1]
 * pThis operation processes the elements one at a time, in encounter
 * order if one exists.  Performing the action for one element
 * a 
 href=../concurrent/package-summary.html#MemoryVisibilityihappens-before/i/a
 * performing the action for subsequent elements, but for any given 
 element,
 * the action may be performed in whatever thread the library chooses.
 *
 * @param action a a href=package-summary.html#NonInterference
 *   non-interfering/a action to perform on the elements
 * @see #forEach(Consumer)
 */
void forEachOrdered(Consumer? super T action);
 



Re: RFR 8029452: Fork/Join task ForEachOps.ForEachOrderedTask clarifications and minor improvements

2014-01-10 Thread Paul Sandoz
On Jan 10, 2014, at 7:11 PM, Mike Duigou mike.dui...@oracle.com wrote:
 
 The second tweak is to consolidate the reporting of elements to within the 
 ForEachOrderedTask.tryComplete method. 
 
 I have also removed the inconsistently applied synchronized block. Either we 
 apply it consistently to reporting or not at all. It was originally there 
 because we were not sure that the happens-before relationship [1] between 
 elements would be guaranteed. However, ForEachOrderedTask sets up such a 
 relationship via completion counts to ensure leaf nodes complete in 
 encounter order (if any) where only one leaf can be completing (which was 
 left most leaf that was not completed), hence stamping a fence in the ground 
 at these point seems redundant (at least i cannot see its value but could be 
 missing something subtle).
 
 Coud not the lock object be removed?
 

Doh, yes, thanks, updated,
Paul.