Uploaded image for project: 'Crucible'
  1. Crucible
  2. CRUC-6940

Poor performance on review creation with large changeset

    XMLWordPrintable

Details

    Description

      Found on 3.6-SNAPSHOT (7313b0b768785bf0de602aba96fc43228c85ab70) when creating projects with many reviews and reviews with big changesets specifically.
      cc lpater

      Discovered that creating a review with 800 changesets took very long. Some profiling with don.willis@atlassian.com pointed out some suspected code in https://fisheye.dev.internal.atlassian.com/browse/~br=master/FE-git/src/java/com/cenqua/crucible/notification/DefaultNotificationManager.java?hb=true

      Removing this code made a very big difference on the review creation time. Same script that creates a review with a big changeset takes:

      • 3.5s without this code, and
      • 20-25s with it.

      Regression

      I have done the same test on released versions fecru 3.4 and 3.5, and found that this is a regression between these
      http://fecru2.dyn.syd.atlassian.com:8034/
      http://fecru2.dyn.syd.atlassian.com:8035/

      Using the script here:
      https://bitbucket.org/atlassian/test-data-generator-backend/branch/fecru-review-performance

      Troublesome changeset with 800 file revisions
      https://bitbucket.org/sbirgisson/testrepo2_public/commits/73011193d926896e9ab921e565a99343715813a8

      fecru 3.4

      sigge:test-data-generator-backend sbirgisson$ time bundle exec ./data-generator ./recipes/fecru_commits.rb 
      /Users/sbirgisson/.rvm/gems/ruby-1.9.3-p448/gems/bundler-1.3.5/lib/bundler/runtime.rb:216: warning: Insecure world writable dir /Users/sbirgisson/Development/fecru/lib/idejars/mysql in PATH, mode 040777
      /Users/sbirgisson/.rvm/gems/ruby-1.9.3-p448/gems/bundler-1.3.5/lib/bundler.rb:284: warning: Insecure world writable dir /Users/sbirgisson/Development/fecru/lib/idejars/mysql in PATH, mode 040777
      Executing recipe ./recipes/fecru_commits.rb...
      Creating review 
      Starting review BIGCHANGE-3
      
      real	0m3.555s
      user	0m0.859s
      sys	0m0.122s
      sigge:test-data-generator-backend sbirgisson$ time bundle exec ./data-generator ./recipes/fecru_commits.rb 
      /Users/sbirgisson/.rvm/gems/ruby-1.9.3-p448/gems/bundler-1.3.5/lib/bundler/runtime.rb:216: warning: Insecure world writable dir /Users/sbirgisson/Development/fecru/lib/idejars/mysql in PATH, mode 040777
      /Users/sbirgisson/.rvm/gems/ruby-1.9.3-p448/gems/bundler-1.3.5/lib/bundler.rb:284: warning: Insecure world writable dir /Users/sbirgisson/Development/fecru/lib/idejars/mysql in PATH, mode 040777
      Executing recipe ./recipes/fecru_commits.rb...
      Creating review 
      Starting review BIGCHANGE-4
      
      real	0m4.755s
      user	0m0.853s
      sys	0m0.120s
      sigge:test-data-generator-backend sbirgisson$ time bundle exec ./data-generator ./recipes/fecru_commits.rb 
      /Users/sbirgisson/.rvm/gems/ruby-1.9.3-p448/gems/bundler-1.3.5/lib/bundler/runtime.rb:216: warning: Insecure world writable dir /Users/sbirgisson/Development/fecru/lib/idejars/mysql in PATH, mode 040777
      /Users/sbirgisson/.rvm/gems/ruby-1.9.3-p448/gems/bundler-1.3.5/lib/bundler.rb:284: warning: Insecure world writable dir /Users/sbirgisson/Development/fecru/lib/idejars/mysql in PATH, mode 040777
      Executing recipe ./recipes/fecru_commits.rb...
      Creating review 
      Starting review BIGCHANGE-5
      
      real	0m3.149s
      user	0m0.865s
      sys	0m0.126s
      
      

      fecru 3.5

      sigge:test-data-generator-backend sbirgisson$ time bundle exec ./data-generator ./recipes/fecru_commits.rb 
      /Users/sbirgisson/.rvm/gems/ruby-1.9.3-p448/gems/bundler-1.3.5/lib/bundler/runtime.rb:216: warning: Insecure world writable dir /Users/sbirgisson/Development/fecru/lib/idejars/mysql in PATH, mode 040777
      /Users/sbirgisson/.rvm/gems/ruby-1.9.3-p448/gems/bundler-1.3.5/lib/bundler.rb:284: warning: Insecure world writable dir /Users/sbirgisson/Development/fecru/lib/idejars/mysql in PATH, mode 040777
      Executing recipe ./recipes/fecru_commits.rb...
      Creating review 
      Starting review BIGCHANGE-5
      
      real	0m23.408s
      user	0m0.856s
      sys	0m0.123s
      sigge:test-data-generator-backend sbirgisson$ time bundle exec ./data-generator ./recipes/fecru_commits.rb 
      /Users/sbirgisson/.rvm/gems/ruby-1.9.3-p448/gems/bundler-1.3.5/lib/bundler/runtime.rb:216: warning: Insecure world writable dir /Users/sbirgisson/Development/fecru/lib/idejars/mysql in PATH, mode 040777
      /Users/sbirgisson/.rvm/gems/ruby-1.9.3-p448/gems/bundler-1.3.5/lib/bundler.rb:284: warning: Insecure world writable dir /Users/sbirgisson/Development/fecru/lib/idejars/mysql in PATH, mode 040777
      Executing recipe ./recipes/fecru_commits.rb...
      Creating review 
      Starting review BIGCHANGE-6
      
      real	0m22.361s
      user	0m0.857s
      sys	0m0.120s
      sigge:test-data-generator-backend sbirgisson$ time bundle exec ./data-generator ./recipes/fecru_commits.rb 
      /Users/sbirgisson/.rvm/gems/ruby-1.9.3-p448/gems/bundler-1.3.5/lib/bundler/runtime.rb:216: warning: Insecure world writable dir /Users/sbirgisson/Development/fecru/lib/idejars/mysql in PATH, mode 040777
      /Users/sbirgisson/.rvm/gems/ruby-1.9.3-p448/gems/bundler-1.3.5/lib/bundler.rb:284: warning: Insecure world writable dir /Users/sbirgisson/Development/fecru/lib/idejars/mysql in PATH, mode 040777
      Executing recipe ./recipes/fecru_commits.rb...
      Creating review 
      Starting review BIGCHANGE-7
      
      real	0m22.498s
      user	0m0.862s
      sys	0m0.125s
      
      

      Possible culprit code

          @EventListener
          public void didReviewEvent(final ReviewEvent event) {
      //        txTemplate.execute(new TxCallback<Void>() {
      //            public Void doInTransaction(TransactionStatus status) throws Exception {
      //                final NotificationEvent note;
      //                final Review review = reviewManager.getReviewByPermaId(event.getReviewId().getId());
      //                if (event instanceof ReviewerCompletedEvent) {
      //                    final ReviewerCompletedEvent e = (ReviewerCompletedEvent) event;
      //                    final User user = userManager.getUser(e.getActioner().getUserName());
      //
      //                    note = new CompletedNotificationEvent(user, review);
      //                } else if (event instanceof ReviewerUncompletedEvent) {
      //                    final ReviewerUncompletedEvent e = (ReviewerUncompletedEvent) event;
      //                    final User user = userManager.getUser(e.getActioner().getUserName());
      //
      //                    note = new UncompletedNotificationEvent(user, review);
      //                } else if (event instanceof AllReviewersCompletedEvent) {
      //                    final AllReviewersCompletedEvent e = (AllReviewersCompletedEvent) event;
      //                    final User user = userManager.getUser(e.getActioner().getUserName());
      //
      //                    note = new AllCompletedNotificationEvent(user, review);
      //                } else if (event instanceof AllReviewersNoLongerCompletedEvent) {
      //                    final AllReviewersNoLongerCompletedEvent e = (AllReviewersNoLongerCompletedEvent) event;
      //                    final User user = userManager.getUser(e.getActioner().getUserName());
      //
      //                    note = new AllUncompletedNotificationEvent(user, review);
      //                } else {
      //                    note = null;
      //                }
      //
      //                if (note != null) {
      //                    session().save(note);
      //                    note.doNotify();
      //                }
      //                return null;
      //            }
      //        });
          }
      

      Attachments

        Issue Links

          Activity

            People

              lpater Lukasz Pater
              sbirgisson Sigurdur Birgisson (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: