You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by "Jake Mannix (JIRA)" <ji...@apache.org> on 2011/09/13 06:41:08 UTC

[jira] [Created] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
---------------------------------------------------------------------------------------------------------------------------

                 Key: GIRAPH-31
                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
             Project: Giraph
          Issue Type: Improvement
          Components: graph
    Affects Versions: 0.70.0
            Reporter: Jake Mannix
            Assignee: Jake Mannix


As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Jake Mannix updated GIRAPH-31:
------------------------------

    Attachment: GIRAPH-31.diff

Updated patch - remove isSorted(), document the fact that the iterator may or may not be sorted (and in fact is, in Vertex), and that users may subclass either Vertex *or* MutableVertex.  

I have not tested subclassing BasicVertex, which I suspect would fail in various ways, as VertexReader, GraphMapper, and some other classes may expect to get a MutableVertex for some methods.

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff, GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Avery Ching commented on GIRAPH-31:
-----------------------------------

Times up (it is 9:10 PM) and there were no comments.  If there are any additional interface changes, we can always address them later.  I made some minor changes to fit the code conventions and verified that unittests passed.

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff, GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Avery Ching commented on GIRAPH-31:
-----------------------------------

committer +1.  A few minor formatting issues (missing javadoc and over 80 char lines - I can fix before committing), but otherwise great!  I agree with Dmitriy's comment that the default should be false.  We should probably wait (maybe a day) for other folks to chime in for this one since it's a user facing interface.

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Jake Mannix commented on GIRAPH-31:
-----------------------------------

Avery, Dmitriy - after thinking about it, I think both true and false are wrong!  BasicVertex shouldn't implement this method at all, leave it abstract, and sublcasses which implement iterator() are forced to also tell users whether it chose to implement it sorted or not.  

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Jake Mannix commented on GIRAPH-31:
-----------------------------------

But I didn't do anything about the uses of the Edge class in VertexChanges, VertexMutations, or hence MutableVertex#addEdgeRequest().  Not sure about their uses, so those are left alone.

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Dmitriy V. Ryaboy commented on GIRAPH-31:
-----------------------------------------

I was just commenting on the javadoc, not the implementation. Though now that you say that, i think you are right, false is a safer thing to do.

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Jake Mannix commented on GIRAPH-31:
-----------------------------------

Right, as many implementations will just 'throw new UnsupportedOperationException("We don't sort!");'

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Jake Mannix commented on GIRAPH-31:
-----------------------------------

+1 to that, given your argument on the current use of the class.  It may come a time when we have generic things going on in GraphMapper or BspServiceWorker which need to do special optimized things to sorted vertices, and at that time we can add an "isSorted()" or "getSortedIterator()" method.

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Avery Ching commented on GIRAPH-31:
-----------------------------------

How about I wait until tonight (say after 7 pm) sometime to commit this?  In case anyone has any last thoughts...

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff, GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Jake Mannix commented on GIRAPH-31:
-----------------------------------

Sounds good to me!  "Lazy consensus" is pretty common to The Apache Way ( http://www.apache.org/foundation/voting.html#LazyConsensus ).

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff, GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Avery Ching commented on GIRAPH-31:
-----------------------------------

You sure you don't want to just provide the interfaces 

Iterator<Edge<I, E>> getOutEdgeIterator();
Iterator<Edge<I, E>> getSortedOutEdgeIterator();

or 

Iterator<I> getOutEdgeIterator();
Iterator<I> getSortedOutEdgeIterator();

It would do away with this issue of sorted...and still keep iterable, but sorted or not, it's up to the implementation.

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Resolved] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Avery Ching resolved GIRAPH-31.
-------------------------------

    Resolution: Fixed

Thanks Jake!

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff, GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Hudson commented on GIRAPH-31:
------------------------------

Integrated in Giraph-trunk-Commit #5 (See [https://builds.apache.org/job/Giraph-trunk-Commit/5/])
    GIRAPH-31: Hide the SortedMap<I, Edge<I,E>> in Vertex from client
visibility (impl. detail), replace with appropriate accessor
methods. jake.mannix via aching.

aching : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1170431
Files : 
* /incubator/giraph/trunk/CHANGELOG
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PseudoRandomVertexInputFormat.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleFailVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleShortestPathsVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleSuperstepVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/MutableVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/Vertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexRange.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexResolver.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/lib/JsonBase64VertexInputFormat.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/lib/JsonBase64VertexOutputFormat.java


> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff, GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Jake Mannix commented on GIRAPH-31:
-----------------------------------

Noticed one more thing: if people do subclass Vertex, we need to change destEdgeMap to be protected, as we don't provide a getter anymore, so subclasses which want to do range-queries or whatnot, can do so.

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Dmitriy V. Ryaboy commented on GIRAPH-31:
-----------------------------------------

Avery,
It seems like requiring all BasicVertex implementations to implement a sorted iterator even when they don't need it is a bit heavy-handed.

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Avery Ching commented on GIRAPH-31:
-----------------------------------

After "iterating" on it, given that we don't have a well defined use case for a sorted iterator and those apis I suggested are a little nasty, I think the prefer the following:

Each Vertex implementation should implement Iterable as you both suggest, but I think following the Java utils style of sorted or not feels the most natural.  We can describe the iterating order via javadoc and we can have multiple Vertex implementation, i.e. SortedPrimitiveVertex, HashPrimitiveVertex, etc.  Somehow isSorted() feels a little yucky.  Examples from java utils Set implemetnations:

TreeSet:
Iterator<E> iterator()  Returns an iterator over the elements in this set in ascending order.

HashSet:
Iterator<E> iterator() Returns an iterator over the elements in this set.

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Jake Mannix updated GIRAPH-31:
------------------------------

    Attachment: GIRAPH-31.diff

This patch is also an attempt at hiding the Edge class from external-facing APIs.

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Claudio Martella commented on GIRAPH-31:
----------------------------------------

One question: how can I provide my own implementation of the edge-containing datastructure if addEdge is final? Maybe we should drop the final?

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Jake Mannix commented on GIRAPH-31:
-----------------------------------

And for the implementations which have both the ability to provide a sorted iterator which isn't prohibitively expensive, but also provide a much faster unsorted iterator, they can choose whether to return true or false from the "isSorted()" method, and provide another method of the type you're suggesting. 


> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-31) Hide the SortedMap> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods

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

Dmitriy V. Ryaboy commented on GIRAPH-31:
-----------------------------------------

non-committer +1.

Please change javadoc for providesSortedIterator to not just say "@return true" -- implementations that override this to return false might forget to provide their own javadoc, inherit this, and this claim behavior opposite from what they actually do.

> Hide the SortedMap<I, Edge<I,E>> in Vertex from client visibility (impl. detail), replace with appropriate accessor methods
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-31
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-31
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>         Attachments: GIRAPH-31.diff
>
>
> As discussed on the list, and on GIRAPH-28, the SortedMap<I, Edge<I,E>> is an implementation detail which needs not be exposed to application developers - they need to iterate over the edges, and possibly access them one-by-one, and remove them (in the Mutable case), but they don't need the SortedMap, and creating primitive-optimized BasicVertex implementations is hampered by the fact that clients expect this Map to exist.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira