You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by Avery Ching <av...@gmail.com> on 2011/12/21 08:50:20 UTC
Review Request: GIRAPH-112: Use elements() properly in
LongDoubleFloatDoubleVertex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3287/
-----------------------------------------------------------
Review request for giraph.
Summary
-------
As pointed out by YuanYua, the array returned by elements() cannot have its length used since the array contains all the elements currently stored in the mahout collections, even including invalid elements between size and capacity.
Whenever possible I converted elements() into forEach(), forEachKey(), forEachPair(). Used size() in other cases.
Fixed some formatting violations as well in LongDoubleFloatDoubleVertex.java.
This addresses bug GIRAPH-112.
https://issues.apache.org/jira/browse/GIRAPH-112
Diffs
-----
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1221634
Diff: https://reviews.apache.org/r/3287/diff
Testing
-------
Local unittests and MR unittests.
Thanks,
Avery
Re: Review Request: GIRAPH-112: Use elements() properly in LongDoubleFloatDoubleVertex
Posted by Avery Ching <ac...@apache.org>.
Thanks for the quick review Sebastian! I think I still need a +1 from a
Giraph committer before I can commit.
Avery
On 12/20/11 11:58 PM, Sebastian Schelter wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3287/
>
>
> Ship it!
>
> I ran into the same issue yesterday and the solution presented here is correct. For reasons of efficiency, list.elements() returns the internal underlying array for the list, which might be bigger than the number of elements stored in the list. Therefore you should only iterate until list.size() or use the foreachKey() callback.
>
> - Sebastian
>
>
> On December 21st, 2011, 7:50 a.m., Avery Ching wrote:
>
> Review request for giraph.
> By Avery Ching.
>
> /Updated 2011-12-21 07:50:20/
>
>
> Description
>
> As pointed out by YuanYua, the array returned by elements() cannot have its length used since the array contains all the elements currently stored in the mahout collections, even including invalid elements between size and capacity.
>
> Whenever possible I converted elements() into forEach(), forEachKey(), forEachPair(). Used size() in other cases.
>
> Fixed some formatting violations as well in LongDoubleFloatDoubleVertex.java.
>
>
> Testing
>
> Local unittests and MR unittests.
>
> *Bugs: * GIRAPH-112 <https://issues.apache.org/jira/browse/GIRAPH-112>
>
>
> Diffs
>
> * http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java
> (1221634)
>
> View Diff <https://reviews.apache.org/r/3287/diff/>
>
Re: Review Request: GIRAPH-112: Use elements() properly in
LongDoubleFloatDoubleVertex
Posted by Sebastian Schelter <ss...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3287/#review4036
-----------------------------------------------------------
Ship it!
I ran into the same issue yesterday and the solution presented here is correct. For reasons of efficiency, list.elements() returns the internal underlying array for the list, which might be bigger than the number of elements stored in the list. Therefore you should only iterate until list.size() or use the foreachKey() callback.
- Sebastian
On 2011-12-21 07:50:20, Avery Ching wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3287/
> -----------------------------------------------------------
>
> (Updated 2011-12-21 07:50:20)
>
>
> Review request for giraph.
>
>
> Summary
> -------
>
> As pointed out by YuanYua, the array returned by elements() cannot have its length used since the array contains all the elements currently stored in the mahout collections, even including invalid elements between size and capacity.
>
> Whenever possible I converted elements() into forEach(), forEachKey(), forEachPair(). Used size() in other cases.
>
> Fixed some formatting violations as well in LongDoubleFloatDoubleVertex.java.
>
>
> This addresses bug GIRAPH-112.
> https://issues.apache.org/jira/browse/GIRAPH-112
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1221634
>
> Diff: https://reviews.apache.org/r/3287/diff
>
>
> Testing
> -------
>
> Local unittests and MR unittests.
>
>
> Thanks,
>
> Avery
>
>
Re: Review Request: GIRAPH-112: Use elements() properly in
LongDoubleFloatDoubleVertex
Posted by Jake Mannix <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3287/#review4071
-----------------------------------------------------------
Ship it!
Good catch, this looks great, thanks!
- Jake
On 2011-12-21 07:50:20, Avery Ching wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3287/
> -----------------------------------------------------------
>
> (Updated 2011-12-21 07:50:20)
>
>
> Review request for giraph.
>
>
> Summary
> -------
>
> As pointed out by YuanYua, the array returned by elements() cannot have its length used since the array contains all the elements currently stored in the mahout collections, even including invalid elements between size and capacity.
>
> Whenever possible I converted elements() into forEach(), forEachKey(), forEachPair(). Used size() in other cases.
>
> Fixed some formatting violations as well in LongDoubleFloatDoubleVertex.java.
>
>
> This addresses bug GIRAPH-112.
> https://issues.apache.org/jira/browse/GIRAPH-112
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1221634
>
> Diff: https://reviews.apache.org/r/3287/diff
>
>
> Testing
> -------
>
> Local unittests and MR unittests.
>
>
> Thanks,
>
> Avery
>
>