You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@flink.apache.org by greghogan <gi...@git.apache.org> on 2015/12/15 13:11:52 UTC

[GitHub] flink pull request: [FLINK-2882] [core] Improve performance of str...

GitHub user greghogan opened a pull request:

    https://github.com/apache/flink/pull/1455

    [FLINK-2882] [core] Improve performance of string conversions

    Memoize string representations of AbstractID.
    
    Use lookup table for byte-to-hex conversion in StringUtils.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/greghogan/flink 2882_improve_performance_of_string_conversions

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/1455.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1455
    
----
commit e23322855f94245783d7f0a5accb1558d2816152
Author: Greg Hogan <co...@greghogan.com>
Date:   2015-12-14T19:47:33Z

    [FLINK-2882] [core] Improve performance of string conversions
    
    Memoize string representations of AbstractID.
    
    Use lookup table for byte-to-hex conversion in StringUtils.

----


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

[GitHub] flink pull request: [FLINK-2882] [core] Improve performance of str...

Posted by fhueske <gi...@git.apache.org>.

Github user fhueske commented on the pull request:

    https://github.com/apache/flink/pull/1455#issuecomment-164880926
  
    Looks good. 1+ for removing `toShortString` in a separate issue.
    I'll merge this PR.


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

[GitHub] flink pull request: [FLINK-2882] [core] Improve performance of str...

Posted by greghogan <gi...@git.apache.org>.

Github user greghogan commented on the pull request:

    https://github.com/apache/flink/pull/1455#issuecomment-164824368
  
    @tillrohrmann There exists StringUtilsTest with small test.
    
    One of the TravisCI errors was a timeout and the other killed by the watchdog. Are timeouts being reworked to use RetryOnException or similar?
    
    @uce It looks like toShortString is used for more than debugging and tests so I'd go for a separate PR.


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

[GitHub] flink pull request: [FLINK-2882] [core] Improve performance of str...

Posted by StephanEwen <gi...@git.apache.org>.

Github user StephanEwen commented on the pull request:

    https://github.com/apache/flink/pull/1455#issuecomment-164790880
  
    Looks very good.
    
    I am wondering if we can get rid of the toShortString method. What is that one actually used for?


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

[GitHub] flink pull request: [FLINK-2882] [core] Improve performance of str...

Posted by tillrohrmann <gi...@git.apache.org>.

Github user tillrohrmann commented on the pull request:

    https://github.com/apache/flink/pull/1455#issuecomment-164790091
  
    Thanks for your contribution @greghogan. Efficiency improvements look really good. The test failures seem to be unrelated.
    
    Would be great if you could add a small test for the StringUtils method.
    
    Apart from that, +1 for merging.


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

[GitHub] flink pull request: [FLINK-2882] [core] Improve performance of str...

Posted by asfgit <gi...@git.apache.org>.

Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/1455


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

[GitHub] flink pull request: [FLINK-2882] [core] Improve performance of str...

Posted by uce <gi...@git.apache.org>.

Github user uce commented on the pull request:

    https://github.com/apache/flink/pull/1455#issuecomment-164802103
  
    Yes, that's good to remove. I pointed that out as well when Greg opened the initial issue.
    
    We can either do it as part of this PR or I can do it as a follow up.


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