[jira] [Commented] (JENA-713) Reduce costly string operations in utility class

["Rob Vesse (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/JENA-713?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14030532#comment-14030532 ] 

Rob Vesse commented on JENA-713:
--------------------------------

Thanks for the patch, the basic idea of switching to use `StringBuilder` makes sense but the patch could do with some cleaning up here and there e.g.

{noformat}
     // Formatting various items
-    public static String stringForTriple(Triple triple)
+    public static String stringForTriple( Triple triple )
     {
-        return
-            stringForNode(triple.getSubject())+" "+
-            stringForNode(triple.getPredicate())+" "+
-            stringForNode(triple.getObject()) ;
+        StringBuilder result = new StringBuilder();
+        stringForNode( result, triple.getSubject() );
+        result.append( " " );
+        stringForNode( triple.getPredicate() );
+        result.append( " " );
+        stringForNode( triple.getObject() );
+        return result.toString();
     }
{noformat}

Here it looks like the predicate and object strings will be thrown away.
{noformat}
     // To a Writer?
     private static void formatTriple(IndentedWriter out, Triple triple, SerializationContext sCxt)
     {
-        out.print(stringForNode(triple.getSubject(), sCxt)) ;
-        out.print(" ") ;
-        out.print(stringForNode(triple.getPredicate(), sCxt)) ;
-        out.print(" ") ;
-        out.print(stringForNode(triple.getObject(), sCxt)) ;
-        out.print(" .") ;
+        StringBuilder result = new StringBuilder(  );
+        stringForNode( result, triple.getSubject(), sCxt );
+        result.append( " ") ;
+        stringForNode( result, triple.getPredicate(), sCxt );
+        result.append( " " ) ;
+        stringForNode(result, triple.getObject(), sCxt);
+        result.append( " ." ) ;
+        out.print(result.toString());
     }
{noformat}

This goes through a writer API so I am not sure why you would switch it out to pre-build the string since in this case you appear to have introduced additional operations?

Since this is a change that affects a low level API relied upon at various places throughout the stack would you mind adding a secondary patch with a set of test cases which verify that behaviour is as expected, there may be existing test cases for the relevant classes that you can simply add to
     

> Reduce costly string operations in utility class
> ------------------------------------------------
>
>                 Key: JENA-713
>                 URL: https://issues.apache.org/jira/browse/JENA-713
>             Project: Apache Jena
>          Issue Type: Improvement
>          Components: ARQ
>            Reporter: Kristian Rosenvold
>         Attachments: jena.patch
>
>
> The enclosed patch improves performance for all kinds of string-serialization of rdf graphs. The patch improves the performance of "UpdateRequest.output" by approx. 30%, whcih makes remote sparql queries  a fair bit faster. The patch will probably improve performance across the board.



--
This message was sent by Atlassian JIRA
(v6.2#6252)