Github user JoshRosen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/7924#discussion_r36327976
  
    --- Diff: 
core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java ---
    @@ -285,6 +310,19 @@ public BytesToBytesMapIterator iterator() {
       }
     
       /**
    +   * Returns an iterator for iterating over the entries of this map and 
sets if it is destructive.
    +   *
    +   * For efficiency, all calls to `next()` will return the same {@link 
Location} object.
    +   *
    +   * If any other lookups or operations are performed on this map while 
iterating over it, including
    +   * `lookup()`, the behavior of the returned iterator is undefined.
    +   */
    +  public BytesToBytesMapIterator iterator(boolean destructive) {
    --- End diff --
    
    I would actually split this into two methods, one called 
`destructiveIterator` and another called just `iterator()`, like what we had 
before.  For the destructive version, I'd add a comment explaining that it's 
illegal to call _any_ other method on the map after the `destructiveIterator` 
method has been called.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to