[jira] [Commented] (SQOOP-2906) Optimization of AvroUtil.toAvroIdentifier

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

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

ASF GitHub Bot commented on SQOOP-2906:
---------------------------------------

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

    https://github.com/apache/sqoop/pull/18#discussion_r59360059
  
    --- Diff: src/java/org/apache/sqoop/avro/AvroUtil.java ---
    @@ -114,11 +114,20 @@ public static String toAvroColumn(String column) {
        * Format candidate to avro specifics
        */
       public static String toAvroIdentifier(String candidate) {
    -    String formattedCandidate = candidate.replaceAll("\\W+", "_");
    -    if (formattedCandidate.substring(0,1).matches("[a-zA-Z_]")) {
    -      return formattedCandidate;
    +    char[] data = candidate.toCharArray();
    +    int stringIndex = 0;
    +
    +    for (char c:data) {
    +      if (Character.isLetterOrDigit(c) || c == '_') {
    +        data[stringIndex++] = c;
    +      }
    +    }
    +
    +    char initial = data[0];
    +    if (Character.isLetter(initial) || initial == '_') {
    +      return new String(data, 0, stringIndex);
    --- End diff --
    
    Your code will first create a char array and then eventually update char in the array. As result you will create another copy as a new String. Have you thought about using a `StringBuilder` directly?
    ```
      final StringBuilder sb = new StringBuilder();
      for (char c : candidate) {
        if (Character.isLetterOrDigit(c) || c == '_') {
          sb.append(c);
        }
      }
      ...
        return sb.toString();
    ```


> Optimization of AvroUtil.toAvroIdentifier
> -----------------------------------------
>
>                 Key: SQOOP-2906
>                 URL: https://issues.apache.org/jira/browse/SQOOP-2906
>             Project: Sqoop
>          Issue Type: Improvement
>            Reporter: Joeri Hermans
>              Labels: avro, hadoop, optimization
>
> Hi all
> Our distributed profiler indicated some inefficiencies in the AvroUtil.toAvroIdentifier method, more specifically, the use of Regex patterns. This can be directly observed from the FlameGraph generated by this profiler (https://jhermans.web.cern.ch/jhermans/sqoop_avro_flamegraph.svg). We implemented an optimization, and compared this with the original method. On our testing machine, the optimization by itself is about 500% (on average) more efficient compared to the original implementation. We have yet to test how this optimization will influence the performance of user jobs.
> Any suggestions or remarks are welcome.
> Kind regards,
> Joeri
> https://github.com/apache/sqoop/pull/18



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)