X10

'at' expression returns original object (not copy)

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: X10 2.1.2
  • Fix Version/s: X10 2.1.2
  • Component/s: XRX Runtime
  • Labels:
    None
  • Number of attachments :
    0

Description

The following program uses an at-expression to copy the array v to a new array w.
It modifies v, and prints elements of w. However, both C++ and Java back ends seem to
get the original array as w, not a copy, when there is one place. 

public class CopyBackFromAtExp {
  public static def spawnAndReturn() {
     val v = [1];
     async {
       for([i] in 1 .. 20) {
         v(0) = i;
         Console.OUT.println("v(0) = " + i);
         }
       }
     return v;
  }

  public static def main(argv:Array[String](1)) {
    val w = at(here.next()) spawnAndReturn();
    for([j] in 1 .. 20) {
      Console.OUT.println("w(0) = " + w(0));
    }
  }
}

C++ output

~/x10/prog-guide/x10.samples/guide/heart-of-concurrency: x10c++ CopyBackFromAtExp.x10
~/x10/prog-guide/x10.samples/guide/heart-of-concurrency: mpirun a.out
w(0) = 1
v(0) = 1
w(0) = 1
v(0) = 2
w(0) = 2
v(0) = 3
w(0) = 3
v(0) = 4
w(0) = 4
v(0) = 5
w(0) = 5
v(0) = 6
w(0) = 6
v(0) = 7
v(0) = 8
w(0) = 7
v(0) = 9
v(0) = 10
w(0) = 9
v(0) = 11
v(0) = 12
w(0) = 11
v(0) = 13
w(0) = 13
v(0) = 14
w(0) = 14
v(0) = 15
w(0) = 15
v(0) = 16
w(0) = 16
v(0) = 17
w(0) = 17
v(0) = 18
w(0) = 18
v(0) = 19
w(0) = 19
v(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20

Java output:

: x10 CopyBackFromAtExp
v(0) = 1
v(0) = 2
v(0) = 3
v(0) = 4
v(0) = 5
v(0) = 6
v(0) = 7
v(0) = 8
v(0) = 9
v(0) = 10
v(0) = 11
v(0) = 12
v(0) = 13
v(0) = 14
v(0) = 15
v(0) = 16
v(0) = 17
v(0) = 18
v(0) = 19
v(0) = 20
w(0) = 1
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
w(0) = 20
~/x10/prog-guide/x10.samples/guide/heart-of-concurrency:

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Bard Bloom added a comment -

The problem is easier to see if we force interleaving: 

/*
 *  This file is part of the X10 project (http://x10-lang.org).
 *
 *  This file is licensed to You under the Eclipse Public License (EPL);
 *  You may not use this file except in compliance with the License.
 *  You may obtain a copy of the License at
 *      http://www.opensource.org/licenses/eclipse-1.0.php
 *
 *  (C) Copyright IBM Corporation 2010.
 */

/**
 * Copying objects back from an 'at'
 */

public class CopyBackFromAtExp {
  public static def spawnAndReturn() {
     val v = [1];
     async {
       for([i] in 1 .. 20) {
         System.sleep(100);
         v(0) = i;
         Console.OUT.println("v(0) = " + i);
         }
       }
     return v;
  }

  public static def main(argv:Array[String](1)) {
    val w = at(here.next()) spawnAndReturn();
    for([j] in 1 .. 20) {
      System.sleep(100);
      Console.OUT.println("w(0) = " + w(0));
    }
  }
}
Show
Bard Bloom added a comment - The problem is easier to see if we force interleaving: /* * This file is part of the X10 project (http: //x10-lang.org). * * This file is licensed to You under the Eclipse Public License (EPL); * You may not use this file except in compliance with the License. * You may obtain a copy of the License at * http: //www.opensource.org/licenses/eclipse-1.0.php * * (C) Copyright IBM Corporation 2010. */ /** * Copying objects back from an 'at' */ public class CopyBackFromAtExp { public static def spawnAndReturn() { val v = [1]; async { for ([i] in 1 .. 20) { System .sleep(100); v(0) = i; Console.OUT.println( "v(0) = " + i); } } return v; } public static def main(argv:Array[ String ](1)) { val w = at(here.next()) spawnAndReturn(); for ([j] in 1 .. 20) { System .sleep(100); Console.OUT.println( "w(0) = " + w(0)); } } }
Hide
Permalink
David Grove added a comment -

there's no finish around your async. so, no reason to expect the updates to happen before the return.

Show
David Grove added a comment - there's no finish around your async. so, no reason to expect the updates to happen before the return.
Hide
Permalink
Bard Bloom added a comment -

That's irrelevant. v and w should be different objects. The output shows that they are the same object – we change v and see that w also changes.

Show
Bard Bloom added a comment - That's irrelevant. v and w should be different objects. The output shows that they are the same object – we change v and see that w also changes.
Hide
Permalink
Igor Peshansky added a comment -

This is either a runtime bug or a code generation bug in both backends. The frontend has nothing to do with copying.

Show
Igor Peshansky added a comment - This is either a runtime bug or a code generation bug in both backends. The frontend has nothing to do with copying.
Hide
Permalink
David Grove added a comment -

It works correctly for two places. I think this is likely to be an improper optimization of at here in XRX.

Show
David Grove added a comment - It works correctly for two places. I think this is likely to be an improper optimization of at here in XRX.
Hide
Permalink
David Grove added a comment -

Olivier, I had a "d'oh" moment on the drive home. In the optimized at, we are deepcopying the environment "on the way in" but I think we are failing to deepcopy the return value of the closure "on the way out". ie the code needs to be:

deepcopy((deepcopy(cls)())
Show
David Grove added a comment - Olivier, I had a "d'oh" moment on the drive home. In the optimized at, we are deepcopying the environment "on the way in" but I think we are failing to deepcopy the return value of the closure "on the way out". ie the code needs to be: deepcopy((deepcopy(cls)())
Hide
Permalink
Olivier Tardieu added a comment -

Dave is correct. Fix committed (r20151).

Show
Olivier Tardieu added a comment - Dave is correct. Fix committed (r20151).
Hide
Permalink
David Grove added a comment -

bulk close of resolved issues from releases prior to 2.2

Show
David Grove added a comment - bulk close of resolved issues from releases prior to 2.2

People

  • Assignee:
    Olivier Tardieu
    Reporter:
    Bard Bloom
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: