davidnavas commented on a change in pull request #24616: [SPARK-27726] [Core] 
Fix performance of ElementTrackingStore deletes when using InMemoryStore under 
high loads
URL: https://github.com/apache/spark/pull/24616#discussion_r285792194

 File path: 
 @@ -248,16 +337,16 @@ private int compare(T e1, T e2, KVTypeInfo.Accessor 
getter) {
           diff = compare(e1, natural, natural.get(e2));
         return diff;
-      } catch (Exception e) {
-        throw Throwables.propagate(e);
+      } catch (ReflectiveOperationException e) {
+        throw new RuntimeException(e);
     private int compare(T e1, KVTypeInfo.Accessor getter, Object v2) {
       try {
         return asKey(getter.get(e1)).compareTo(asKey(v2));
 Review comment:
   I have not attempted to apply the basic trick in getPredicate (of calling or 
not calling asKey() depending on whether the index requires it).  I agree that 
there might be a win there, but the application seems difficult and unlikely to 
yield clear code.  getPredicate() is doing two things -- it's converting a 
Collection into a Set as well as converting all the entries through asKey as 
necessary, AND it's removing the necessity of calling getClass.isArray when it 
   There appear three basic times where this strategy would be useful -- during 
copyElements when parent is defined, and during iteration when first and/or 
last are defined.  Well, I don't see a contract about when first or last can be 
changed wrt when an iteration is started or running, so I'm a little worried 
about modifying that code, and I'm not sure what the usecases are for parents.  
But let's assume that first and last are actually fixed during iteration (it 
seems bad that it is not), and I'm willing to make changes to the parent code 
blind ....  Presumably I'd want a set of compare() routines that took a 
Comparable as the second argument (the second argument being fixed locally to 
asKey(first|last|parent)) with two potential names -- one that would call asKey 
and one that wouldn't.  I'm blanking on ideas for names.  Ideas?
   What I'll do is create the "dumb" version of this compare call, and either 
we'll revert because we want to allow first and last to be modified during 
iteration, or we'll come up with some good names for dealing with the other 
half of the asKey calls.  Or we'll decide that the getClass.isArray calls for 
only one side of the compare isn't so bad.

This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:

With regards,
Apache Git Services

To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to