You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by "Yuanyuan Tian (Created) (JIRA)" <ji...@apache.org> on 2012/01/18 02:53:39 UTC

[jira] [Created] (GIRAPH-125) Bug in LongDoubleFloatDoubleVertex.sendMsgToAllEdges()

Bug in LongDoubleFloatDoubleVertex.sendMsgToAllEdges()
------------------------------------------------------

                 Key: GIRAPH-125
                 URL: https://issues.apache.org/jira/browse/GIRAPH-125
             Project: Giraph
          Issue Type: Bug
          Components: graph
    Affects Versions: 0.1.0
            Reporter: Yuanyuan Tian


I just found a bug in the sendMsgToAllEdges() function of the LongDoubleFloatDoubleVertex class. The segment of the code that contains the bug is:

        final LongWritable destVertex = new LongWritable();
        final MutableVertex<LongWritable, DoubleWritable, FloatWritable,
            DoubleWritable> vertex = this;
        verticesWithEdgeValues.forEachKey(new LongProcedure() {
            @Override
            public boolean apply(long destVertexId) {
                destVertex.set(destVertexId);
                vertex.sendMsg(destVertex, msg);
                return true;
            }
        });

Here destVertex is a final object, but this single object is reused in the forEachKey function many times. Each time its actual value is changed but the same object is put to the underlying message list (a hashmap) through vertex.sendMsg. Because the single destVertex object has been put into the underlying hashmap again and again, destVertex.set(destVertexId) will change the existing keys in the hashmap. Eventually, every keys added to the hash map will have the same value as the last key. 

A simple fix is as follows:

        final MutableVertex<LongWritable, DoubleWritable, FloatWritable,
            DoubleWritable> vertex = this;
        verticesWithEdgeValues.forEachKey(new LongProcedure() {
            @Override
            public boolean apply(long destVertexId) {
                vertex.sendMsg(new LongWritable(destVertexId), msg);
                return true;
            }
        });

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (GIRAPH-125) Bug in LongDoubleFloatDoubleVertex.sendMsgToAllEdges()

Posted by "Yuanyuan Tian (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/GIRAPH-125?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Yuanyuan Tian updated GIRAPH-125:
---------------------------------

    Attachment: LongDoubleFloatDoubleVertex.java.patch
    
> Bug in LongDoubleFloatDoubleVertex.sendMsgToAllEdges()
> ------------------------------------------------------
>
>                 Key: GIRAPH-125
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-125
>             Project: Giraph
>          Issue Type: Bug
>          Components: graph
>    Affects Versions: 0.1.0
>            Reporter: Yuanyuan Tian
>            Assignee: Yuanyuan Tian
>              Labels: patch
>             Fix For: 0.1.0
>
>         Attachments: LongDoubleFloatDoubleVertex.java.patch
>
>   Original Estimate: 5m
>  Remaining Estimate: 5m
>
> I just found a bug in the sendMsgToAllEdges() function of the LongDoubleFloatDoubleVertex class. The segment of the code that contains the bug is:
>         final LongWritable destVertex = new LongWritable();
>         final MutableVertex<LongWritable, DoubleWritable, FloatWritable,
>             DoubleWritable> vertex = this;
>         verticesWithEdgeValues.forEachKey(new LongProcedure() {
>             @Override
>             public boolean apply(long destVertexId) {
>                 destVertex.set(destVertexId);
>                 vertex.sendMsg(destVertex, msg);
>                 return true;
>             }
>         });
> Here destVertex is a final object, but this single object is reused in the forEachKey function many times. Each time its actual value is changed but the same object is put to the underlying message list (a hashmap) through vertex.sendMsg. Because the single destVertex object has been put into the underlying hashmap again and again, destVertex.set(destVertexId) will change the existing keys in the hashmap. Eventually, every keys added to the hash map will have the same value as the last key. 
> A simple fix is as follows:
>         final MutableVertex<LongWritable, DoubleWritable, FloatWritable,
>             DoubleWritable> vertex = this;
>         verticesWithEdgeValues.forEachKey(new LongProcedure() {
>             @Override
>             public boolean apply(long destVertexId) {
>                 vertex.sendMsg(new LongWritable(destVertexId), msg);
>                 return true;
>             }
>         });

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-125) Bug in LongDoubleFloatDoubleVertex.sendMsgToAllEdges()

Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/GIRAPH-125?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13188870#comment-13188870 ] 

Hudson commented on GIRAPH-125:
-------------------------------

Integrated in Giraph-trunk-Commit #64 (See [https://builds.apache.org/job/Giraph-trunk-Commit/64/])
    GIRAPH-125: Bug in LongDoubleFloatDoubleVertex.sendMsgToAllEdges().
(humming80 via aching)

aching : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1233107
Files : 
* /incubator/giraph/trunk/CHANGELOG
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java

                
> Bug in LongDoubleFloatDoubleVertex.sendMsgToAllEdges()
> ------------------------------------------------------
>
>                 Key: GIRAPH-125
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-125
>             Project: Giraph
>          Issue Type: Bug
>          Components: graph
>    Affects Versions: 0.1.0
>            Reporter: Yuanyuan Tian
>            Assignee: Yuanyuan Tian
>              Labels: patch
>             Fix For: 0.1.0
>
>         Attachments: LongDoubleFloatDoubleVertex.java.patch
>
>   Original Estimate: 5m
>  Remaining Estimate: 5m
>
> I just found a bug in the sendMsgToAllEdges() function of the LongDoubleFloatDoubleVertex class. The segment of the code that contains the bug is:
>         final LongWritable destVertex = new LongWritable();
>         final MutableVertex<LongWritable, DoubleWritable, FloatWritable,
>             DoubleWritable> vertex = this;
>         verticesWithEdgeValues.forEachKey(new LongProcedure() {
>             @Override
>             public boolean apply(long destVertexId) {
>                 destVertex.set(destVertexId);
>                 vertex.sendMsg(destVertex, msg);
>                 return true;
>             }
>         });
> Here destVertex is a final object, but this single object is reused in the forEachKey function many times. Each time its actual value is changed but the same object is put to the underlying message list (a hashmap) through vertex.sendMsg. Because the single destVertex object has been put into the underlying hashmap again and again, destVertex.set(destVertexId) will change the existing keys in the hashmap. Eventually, every keys added to the hash map will have the same value as the last key. 
> A simple fix is as follows:
>         final MutableVertex<LongWritable, DoubleWritable, FloatWritable,
>             DoubleWritable> vertex = this;
>         verticesWithEdgeValues.forEachKey(new LongProcedure() {
>             @Override
>             public boolean apply(long destVertexId) {
>                 vertex.sendMsg(new LongWritable(destVertexId), msg);
>                 return true;
>             }
>         });

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-125) Bug in LongDoubleFloatDoubleVertex.sendMsgToAllEdges()

Posted by "Avery Ching (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/GIRAPH-125?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13188331#comment-13188331 ] 

Avery Ching commented on GIRAPH-125:
------------------------------------

Thanks for findings this.  I agree this is a bug.  I know that it is small, but do you mind making a patch?  Then I can commit it on your behalf.
                
> Bug in LongDoubleFloatDoubleVertex.sendMsgToAllEdges()
> ------------------------------------------------------
>
>                 Key: GIRAPH-125
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-125
>             Project: Giraph
>          Issue Type: Bug
>          Components: graph
>    Affects Versions: 0.1.0
>            Reporter: Yuanyuan Tian
>   Original Estimate: 5m
>  Remaining Estimate: 5m
>
> I just found a bug in the sendMsgToAllEdges() function of the LongDoubleFloatDoubleVertex class. The segment of the code that contains the bug is:
>         final LongWritable destVertex = new LongWritable();
>         final MutableVertex<LongWritable, DoubleWritable, FloatWritable,
>             DoubleWritable> vertex = this;
>         verticesWithEdgeValues.forEachKey(new LongProcedure() {
>             @Override
>             public boolean apply(long destVertexId) {
>                 destVertex.set(destVertexId);
>                 vertex.sendMsg(destVertex, msg);
>                 return true;
>             }
>         });
> Here destVertex is a final object, but this single object is reused in the forEachKey function many times. Each time its actual value is changed but the same object is put to the underlying message list (a hashmap) through vertex.sendMsg. Because the single destVertex object has been put into the underlying hashmap again and again, destVertex.set(destVertexId) will change the existing keys in the hashmap. Eventually, every keys added to the hash map will have the same value as the last key. 
> A simple fix is as follows:
>         final MutableVertex<LongWritable, DoubleWritable, FloatWritable,
>             DoubleWritable> vertex = this;
>         verticesWithEdgeValues.forEachKey(new LongProcedure() {
>             @Override
>             public boolean apply(long destVertexId) {
>                 vertex.sendMsg(new LongWritable(destVertexId), msg);
>                 return true;
>             }
>         });

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira