You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flex.apache.org by SlavaRa <gi...@git.apache.org> on 2015/10/21 15:11:14 UTC

[GitHub] flex-sdk pull request: Replace with chained append() calls

GitHub user SlavaRa opened a pull request:

    https://github.com/apache/flex-sdk/pull/24

    Replace with chained append() calls

    

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

    $ git pull https://github.com/SlavaRa/flex-sdk feature/remove_string_concatenation_as_argument_to_StringBuffer.appned()_call

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

    https://github.com/apache/flex-sdk/pull/24.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 #24
    
----
commit 086a61665cf5814a2561db8ac62ad92f1056bdef
Author: SlavaRa <ra...@ya.ru>
Date:   2015-10-21T13:08:47Z

    Replace with chained append() calls

----


---
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] flex-sdk pull request: Replace string concatenation as argument St...

Posted by Harbs <gi...@git.apache.org>.
Github user Harbs commented on the pull request:

    https://github.com/apache/flex-sdk/pull/24#issuecomment-152503636
  
    I'd think the compiler optimizes the string concatenation, so there's no difference in performance:
    http://stackoverflow.com/questions/1532461/stringbuilder-vs-string-concatenation-in-tostring-in-java
    
    Is that not the case?
    
    I think "looks bad" is a personal preference. Simple string concatenation is definitely more legible.


---
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.
---

Re: [GitHub] flex-sdk pull request: Replace string concatenation as argument St...

Posted by Alex Harui <ah...@adobe.com>.
Hi SlavaRa,

Do you have a link that supports your claim that the compiler cannot
generate the correct optimization?  This article seems to imply that it
can [1]

IMO, the string concatenation is easier to read.  I would want to see
actual profiling that shows that the string concatenation is affecting
compile times in a significant way.

But the most important question to me is:  Is there a reason you want to
work on the mxmlc compiler?  The “falcon” compiler in the flex-falcon repo
is under much more active development and it would be lower risk to be
doing optimizations in that code base.  The “falcon” compiler is the
compiler for the new FlexJS framework and I believe there is still a
reasonable chance it could replace the mxmlc compiler in Flex SDKs some
day.

-Alex

[1] 
http://stackoverflow.com/questions/1532461/stringbuilder-vs-string-concaten
ation-in-tostring-in-java

On 10/30/15, 5:00 AM, "SlavaRa" <gi...@git.apache.org> wrote:

>Github user SlavaRa commented on the pull request:
>
>    https://github.com/apache/flex-sdk/pull/24#issuecomment-152506154
>  
>    "I think "looks bad" is a personal preference."
>    Perhaps, but in this situation, combined the two approaches
>StringBuffer.append and string concatenations.. in this case, the
>compiler can not generate correct optimization.
>
>
>
>---
>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] flex-sdk pull request: Replace string concatenation as argument St...

Posted by SlavaRa <gi...@git.apache.org>.
Github user SlavaRa commented on the pull request:

    https://github.com/apache/flex-sdk/pull/24#issuecomment-152506154
  
    "I think "looks bad" is a personal preference."
    Perhaps, but in this situation, combined the two approaches StringBuffer.append and string concatenations.. in this case, the compiler can not generate correct optimization.



---
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] flex-sdk pull request: Replace string concatenation as argument St...

Posted by aharui <gi...@git.apache.org>.
Github user aharui commented on the pull request:

    https://github.com/apache/flex-sdk/pull/24#issuecomment-152585318
  
    Hi SlavaRa,
    
    (Apologies if this gets posted twice.  I think email replies as comments isn't working)
    
    Do you have a link that supports your claim that the compiler cannot
    generate the correct optimization?  This article seems to imply that it
    can [1]
    
    IMO, the string concatenation is easier to read.  I would want to see
    actual profiling that shows that the string concatenation is affecting
    compile times in a significant way.
    
    But the most important question to me is:  Is there a reason you want to
    work on the mxmlc compiler?  The “falcon” compiler in the flex-falcon repo
    is under much more active development and it would be lower risk to be
    doing optimizations in that code base.  The “falcon” compiler is the
    compiler for the new FlexJS framework and I believe there is still a
    reasonable chance it could replace the mxmlc compiler in Flex SDKs some
    day.
    
    -Alex
    
    [1]
    http://stackoverflow.com/questions/1532461/stringbuilder-vs-string-concatenation-in-tostring-in-java



---
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] flex-sdk pull request: Replace string concatenation as argument St...

Posted by SlavaRa <gi...@git.apache.org>.
Github user SlavaRa commented on the pull request:

    https://github.com/apache/flex-sdk/pull/24#issuecomment-150963665
  
    https://issues.apache.org/jira/browse/FLEX-34946


---
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] flex-sdk pull request: Replace string concatenation as argument St...

Posted by tomchiverton <gi...@git.apache.org>.
Github user tomchiverton commented on the pull request:

    https://github.com/apache/flex-sdk/pull/24#issuecomment-152466884
  
    I don't see any reasoning behind why this was done - is this approach more performant or something ?


---
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] flex-sdk pull request: Replace string concatenation as argument St...

Posted by SlavaRa <gi...@git.apache.org>.
Github user SlavaRa closed the pull request at:

    https://github.com/apache/flex-sdk/pull/24


---
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] flex-sdk pull request: Replace string concatenation as argument St...

Posted by SlavaRa <gi...@git.apache.org>.
Github user SlavaRa commented on the pull request:

    https://github.com/apache/flex-sdk/pull/24#issuecomment-152467858
  
    I would like to help in the development of the compiler.
    But there is no desire to work with code that looks bad to say the least.
    Using string concatenation contradicts the sense of using StringBuffer.
    And yes, this has a negative impact on performance.


---
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.
---