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

    https://github.com/apache/jena/pull/157#discussion_r71167748
  
    --- Diff: 
jena-arq/src/main/java/org/apache/jena/atlas/data/SortedDataBag.java ---
    @@ -69,19 +69,81 @@
         
         protected final ThresholdPolicy<E> policy;
         protected final SerializationFactory<E> serializationFactory;
    -    protected final Comparator<? super E> comparator;
    +    protected final CanAbortComparator comparator;
         
         protected boolean finishedAdding = false;
         protected boolean spilled = false;
         protected boolean closed = false;
    +    protected volatile boolean cancelled;
         
         public SortedDataBag(ThresholdPolicy<E> policy, 
SerializationFactory<E> serializerFactory, Comparator<? super E> comparator)
         {
             this.policy = policy;
             this.serializationFactory = serializerFactory;
    -        this.comparator = comparator;
    +        this.comparator = new CanAbortComparator(comparator);
         }
         
    +    private final class CanAbortComparator implements Comparator<E> 
    +           {
    +           /**
    +               The test for whether the sort has been cancelled is
    +               performed every <code>cancelTestFrequency</code> 
comparisons.
    +               This reduces the (presumed) overhead of access to a
    +               volatile boolean.               
    +           */
    +           static final int cancelTestFrequency = 10000;
    +           
    +           /**
    +               Count of the number of times this comparator has been 
called.
    +           */
    +           int count = 0;
    +           
    +           final Comparator<? super E> baseComparator;
    +           
    +           public CanAbortComparator(Comparator<? super E> comparator) 
    +                   {
    +                   this.baseComparator = comparator;
    +                   }
    +
    +           @Override public int compare(E o1, E o2) 
    +           {       
    +                   count += 1;
    +                   if (count % cancelTestFrequency == 0) 
    +                   {
    +                           if (cancelled) throw new AbandonSort();
    +                   }
    +                   return baseComparator.compare(o1, o2);
    +           }
    +           
    +           /**
    +               Sort the array <code>e</code> using this comparator
    +                   with the additional ability to abort the sort.
    +           */
    +           public boolean abortableSort(E[] e) {
    +                   try { Arrays.sort(e, this); }
    +                   catch (AbandonSort s) { return true; }
    --- End diff --
    
    Maybe it's just me, but this seems a little backwardsish. I would expect 
`true` to indicate successful completion. Maybe it's better to introduce a 
simple `enum Finish { COMPLETED, ABORTED }` here, for clarity?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to