Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: JRuby 1.6RC1, JRuby 1.6RC2
    • Fix Version/s: JRuby 1.6RC3
    • Component/s: None
    • Labels:
      None
    • Environment:
      Mac OS X 10.6
    • Number of attachments :
      0

      Description

      The rendering of rails views is consistently 4x slower in JRuby 1.6 vs JRuby 1.5.6

      JRuby 1.5.6:

      Rendered config/systems/Financial/views/cases/_address_fields.html.haml (116.0ms)
      Rendered config/systems/Financial/views/cases/_consumer_fields.html.haml (250.0ms)
      Rendered config/systems/Financial/views/cases/_address_fields.html.haml (77.0ms)
      Rendered config/systems/Financial/views/cases/_consumer_fields.html.haml (75.0ms)
      Rendered config/systems/Financial/views/cases/_applicants_infos.html.haml (510.0ms)
      

      Jruby 1.6.0RC:

      Rendered config/systems/Financial/views/cases/_address_fields.html.haml (467.0ms)
      Rendered config/systems/Financial/views/cases/_consumer_fields.html.haml (1045.0ms)
      Rendered config/systems/Financial/views/cases/_address_fields.html.haml (439.0ms)
      Rendered config/systems/Financial/views/cases/_consumer_fields.html.haml (506.0ms)
      Rendered config/systems/Financial/views/cases/_applicants_infos.html.haml (2054.0ms)
      

        Activity

        Hide
        Charles Oliver Nutter added a comment -

        Nick: Yes, that's actually a good idea. RaiseException already overrides fillInStackTrace, so that much is already done. The additional work would be to add a way for exception subclasses to disable backtraces. I mentioned this on the i18n github issue too. It would work similar to fillInStackTrace, with "raise" checking for something like "create_backtrace" on the exception object and using that instead of normal generation.

        I am of two minds on making such an enhancement. I believe it would largely solve the issue of exceptions used as flow control having too much performance overhead, and they would then have no more of a performance hit than catch/throw. But we would have to get it approved in regular Ruby, which has a smaller (but still very large) perf hit from backtraces, and we would in a way be encouraging (or at least facilitating) the use of exceptions for flow control, something that's very rarely a good pattern (and by saying "very rarely", perhaps I answered my own question...sometimes there's no other way, and it would be nice to opt-out of expensive backtraces).

        In any case, if we don't want to roll back the perf gains in JRuby that the new backtrace logic has given us, we'll have to get users to change their code and habits. It is an unfortunate turn of events

        Sven: Do you have an example of using the Cache module? Does it avoid this heavy raising of exceptions? It might be an excellent userland workaround for backtrace performance issues on newer implementations.

        Show
        Charles Oliver Nutter added a comment - Nick: Yes, that's actually a good idea. RaiseException already overrides fillInStackTrace, so that much is already done. The additional work would be to add a way for exception subclasses to disable backtraces. I mentioned this on the i18n github issue too. It would work similar to fillInStackTrace, with "raise" checking for something like "create_backtrace" on the exception object and using that instead of normal generation. I am of two minds on making such an enhancement. I believe it would largely solve the issue of exceptions used as flow control having too much performance overhead, and they would then have no more of a performance hit than catch/throw. But we would have to get it approved in regular Ruby, which has a smaller (but still very large) perf hit from backtraces, and we would in a way be encouraging (or at least facilitating) the use of exceptions for flow control, something that's very rarely a good pattern (and by saying "very rarely", perhaps I answered my own question...sometimes there's no other way, and it would be nice to opt-out of expensive backtraces). In any case, if we don't want to roll back the perf gains in JRuby that the new backtrace logic has given us, we'll have to get users to change their code and habits. It is an unfortunate turn of events Sven: Do you have an example of using the Cache module? Does it avoid this heavy raising of exceptions? It might be an excellent userland workaround for backtrace performance issues on newer implementations.
        Hide
        Thomas E Enebo added a comment -

        If we are unable to get ruby-core to support this we can always make a gem which exposes an API which just does what MRI would do today and we would call our enhanced cheap syntax. Having a gem would also be self-documenting and also probably be self-advertising once we get a few larger projects using it.

        Show
        Thomas E Enebo added a comment - If we are unable to get ruby-core to support this we can always make a gem which exposes an API which just does what MRI would do today and we would call our enhanced cheap syntax. Having a gem would also be self-documenting and also probably be self-advertising once we get a few larger projects using it.
        Hide
        Jean-Dominique Morani added a comment -

        I just wanted to confirm that Sven latest changes to the i18n solve the performance issue. More info at : https://github.com/svenfuchs/i18n/issues/#issue/82/comment/820979

        Show
        Jean-Dominique Morani added a comment - I just wanted to confirm that Sven latest changes to the i18n solve the performance issue. More info at : https://github.com/svenfuchs/i18n/issues/#issue/82/comment/820979
        Hide
        Charles Oliver Nutter added a comment -

        Actually I (re)discovered that the three-argument form of Kernel#raise can be used to short-circuit backtrace generation:

        raise SomeException, some_arg, nil

        Here, some_arg will be pass to SomeException#initialize, and nil will be used for the backtrace. In JRuby 1.6.0.RC3 and higher, we will then not generate a backtrace, cutting out the performance hit you see here.

        It obviously still requires people to adapt their code, but at least we have a way to opt-out of backtrace generation without adding anything to Ruby. Yay!

        I'm going to mark this resolved/fixed, since we have done a little work, and there's no further improvement possible.

        I did some cleanup in recent commits to refactor backtrace logic to try to reduce the perf hit as much as possible. On my machine, it takes about 1.5ms now to generate a full-length backtrace with default JVM settings, with the manipulation we do deferred until the backtrace is actually needed. I also made the modification that the three-arg Kernel#raise does not generate an internal backtrace anymore, so it will be a good workaround.

        I also still plan to write up a blog post on all this, to educate Rubyists and give them some new tools to avoid the bad pattern of exceptions-as-flow-control, or to use the opt-out if they really must use exceptions this way.

        Show
        Charles Oliver Nutter added a comment - Actually I (re)discovered that the three-argument form of Kernel#raise can be used to short-circuit backtrace generation: raise SomeException, some_arg, nil Here, some_arg will be pass to SomeException#initialize, and nil will be used for the backtrace. In JRuby 1.6.0.RC3 and higher, we will then not generate a backtrace, cutting out the performance hit you see here. It obviously still requires people to adapt their code, but at least we have a way to opt-out of backtrace generation without adding anything to Ruby. Yay! I'm going to mark this resolved/fixed, since we have done a little work, and there's no further improvement possible. I did some cleanup in recent commits to refactor backtrace logic to try to reduce the perf hit as much as possible. On my machine, it takes about 1.5ms now to generate a full-length backtrace with default JVM settings, with the manipulation we do deferred until the backtrace is actually needed. I also made the modification that the three-arg Kernel#raise does not generate an internal backtrace anymore, so it will be a good workaround. I also still plan to write up a blog post on all this, to educate Rubyists and give them some new tools to avoid the bad pattern of exceptions-as-flow-control, or to use the opt-out if they really must use exceptions this way.
        Hide
        Sven Fuchs added a comment -

        On i18n's side this should be fixed in 0.6.0: https://github.com/svenfuchs/i18n/issues/82

        Thanks, guys!

        Show
        Sven Fuchs added a comment - On i18n's side this should be fixed in 0.6.0: https://github.com/svenfuchs/i18n/issues/82 Thanks, guys!

          People

          • Assignee:
            Charles Oliver Nutter
            Reporter:
            Jean-Dominique Morani
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: