BigW Consortium Gitlab

  1. 20 Sep, 2016 5 commits
    • Use the `IssuableBaseService` lifecycle hooks to cache `MergeRequestsClosingIssues` · 68e1b5bb
      Timothy Andrew authored
      - Instead of overriding `create` and `update` in `MergeRequests::BaseService`
      - Get all merge request service specs passing
    • Implement a second round of review comments from @DouweM. · 918e589c
      Timothy Andrew authored
      - Don't use `TableReferences` - using `.arel_table` is shorter!
      - Move some database-related code to `Gitlab::Database`
      - Remove the `MergeRequest#issues_closed` and
        `Issue#closed_by_merge_requests`  associations. They were either
        shadowing or were too similar to existing methods. They are not being
        used anywhere, so it's better to remove them to reduce confusion.
      - Use Rails 3-style validations
      - Index for `MergeRequest::Metrics#first_deployed_to_production_at`
      - Only include `CycleAnalyticsHelpers::TestGeneration` for specs that
        need it.
      - Other minor refactorings.
    • Implement (some) comments from @DouweM's review. · 71d4bf72
      Timothy Andrew authored
      - Move things common to `Issue` and `MergeRequest` into `Issuable`
      - Move more database-specific functions into `Gitlab::Database`
      - Indentation changes and other minor refactorings.
    • Implement review comments from @yorickpeterse · 8957293d
      Timothy Andrew authored
      1. Change multiple updates to a single `update_all`
      
      2. Use cascading deletes
      
      3. Extract an average function for the database median.
      
      4. Move database median to `lib/gitlab/database`
      
      5. Use `delete_all` instead of `destroy_all`
      
      6. Minor refactoring
    • Miscellaneous cycle-analytics-related changes. · b43d3af7
      Timothy Andrew authored
      1. Add indexes to `CreateMergeRequestsClosingIssues` columns.
      2. Remove an extraneous `check_if_open` check that is redundant now.
      
      It would've been better to rebase this in, but that's not possible
      because more people are working on this branch.
  2. 19 Sep, 2016 1 commit
  3. 15 Sep, 2016 1 commit
    • Improve performance of the cycle analytics page. · ba25e2f1
      Timothy Andrew authored
      1. These changes bring down page load time for 100 issues from more than
         a minute to about 1.5 seconds.
      
      2. This entire commit is composed of these types of performance
         enhancements:
      
           - Cache relevant data in `IssueMetrics` wherever possible.
           - Cache relevant data in `MergeRequestMetrics` wherever possible.
           - Preload metrics
      
      3. Given these improvements, we now only need to make 4 SQL calls:
      
          - Load all issues
          - Load all merge requests
          - Load all metrics for the issues
          - Load all metrics for the merge requests
      
      4. A list of all the data points that are now being pre-calculated:
      
          a. The first time an issue is mentioned in a commit
      
            - In `GitPushService`, find all issues mentioned by the given commit
              using `ReferenceExtractor`. Set the `first_mentioned_in_commit_at`
              flag for each of them.
      
            - There seems to be a (pre-existing) bug here - files (and
              therefore commits) created using the Web CI don't have
              cross-references created, and issues are not closed even when
              the commit title is "Fixes #xx".
      
          b. The first time a merge request is deployed to production
      
            When a `Deployment` is created, find all merge requests that
            were merged in before the deployment, and set the
            `first_deployed_to_production_at` flag for each of them.
      
          c. The start / end time for a merge request pipeline
      
            Hook into the `Pipeline` state machine. When the `status` moves to
            `running`, find the merge requests whose tip commit matches the
            pipeline, and record the `latest_build_started_at` time for each
            of them. When the `status` moves to `success`, record the
            `latest_build_finished_at` time.
      
          d. The merge requests that close an issue
      
            - This was a big cause of the performance problems we were having
              with Cycle Analytics. We need to use `ReferenceExtractor` to make
              this calculation, which is slow when we have to run it on a large
              number of merge requests.
      
            - When a merge request is created, updated, or refreshed, find the
              issues it closes, and create an instance of
              `MergeRequestsClosingIssues`, which acts as a join model between
              merge requests and issues.
      
            - If a `MergeRequestsClosingIssues` instance links a merge request
              and an issue, that issue closes that merge request.
      
      5. The `Queries` module was changed into a class, so we can cache the
         results of `issues` and `merge_requests_closing_issues` across
         various cycle analytics stages.
      
      6. The code added in this commit is untested. Tests will be added in the
         next commit.
  4. 26 Aug, 2016 2 commits
  5. 15 Aug, 2016 5 commits
  6. 08 Aug, 2016 1 commit
    • Added concern for a faster "cache_key" method · 77c8520e
      Yorick Peterse authored
      This concern provides an optimized/simplified version of the "cache_key"
      method. This method is about 9 times faster than the default "cache_key"
      method.
      
      The produced cache keys _are_ different from the previous ones but this
      is worth the performance improvement. To showcase this I set up a
      benchmark (using benchmark-ips) that compares FasterCacheKeys#cache_key
      with the regular cache_key. The output of this benchmark was:
      
          Calculating -------------------------------------
                     cache_key     4.825k i/100ms
                cache_key_fast    21.723k i/100ms
          -------------------------------------------------
                     cache_key     59.422k (± 7.2%) i/s -    299.150k
                cache_key_fast    543.243k (± 9.2%) i/s -      2.694M
      
          Comparison:
                cache_key_fast:   543243.4 i/s
                     cache_key:    59422.0 i/s - 9.14x slower
      
      To see the impact on real code I applied these changes and benchmarked
      Issue#referenced_merge_requests. For an issue referencing 10 merge
      requests these changes shaved off between 40 and 60 milliseconds.
  7. 29 Jul, 2016 1 commit
    • Method for returning issues readable by a user · 002ad215
      Yorick Peterse authored
      The method Ability.issues_readable_by_user takes a list of users and an
      optional user and returns an Array of issues readable by said user. This
      method in turn is used by
      Banzai::ReferenceParser::IssueParser#nodes_visible_to_user so this
      method no longer needs to get all the available abilities just to check
      if a user has the "read_issue" ability.
      
      To test this I benchmarked an issue with 222 comments on my development
      environment. Using these changes the time spent in nodes_visible_to_user
      was reduced from around 120 ms to around 40 ms.
  8. 27 Jul, 2016 1 commit
  9. 20 Jul, 2016 1 commit
  10. 01 Jul, 2016 1 commit
  11. 29 Jun, 2016 1 commit
  12. 24 Jun, 2016 2 commits
  13. 18 Jun, 2016 1 commit
  14. 16 Jun, 2016 2 commits
  15. 13 Jun, 2016 2 commits
  16. 06 Jun, 2016 1 commit
  17. 03 Jun, 2016 2 commits
  18. 01 Jun, 2016 1 commit
    • Refactor Participable · 580d2501
      Yorick Peterse authored
      There are several changes to this module:
      
      1. The use of an explicit stack in Participable#participants
      2. Proc behaviour has been changed
      3. Batch permissions checking
      
      == Explicit Stack
      
      Participable#participants no longer uses recursion to process "self" and
      all child objects, instead it uses an Array and processes objects in
      breadth-first order. This allows us to for example create a single
      Gitlab::ReferenceExtractor instance and pass this to any Procs. Re-using
      a ReferenceExtractor removes the need for running potentially many SQL
      queries every time a Proc is called on a new object.
      
      == Proc Behaviour Changed
      
      Previously a Proc in Participable was expected to return an Array of
      User instances. This has been changed and instead it's now expected that
      a Proc modifies the Gitlab::ReferenceExtractor passed to it. The return
      value of the Proc is ignored.
      
      == Permissions Checking
      
      The method Participable#participants uses
      Ability.users_that_can_read_project to check if the returned users have
      access to the project of "self" _without_ running multiple SQL queries
      for every user.
  19. 09 May, 2016 1 commit
    • Remove the annotate gem and delete old annotations · f1479b56
      Jeroen van Baarsen authored
      In 8278b763 the default behaviour of annotation
      has changes, which was causing a lot of noise in diffs. We decided in #17382
      that it is better to get rid of the whole annotate gem, and instead let people
      look at schema.rb for the columns in a table.
      
      Fixes: #17382
  20. 06 May, 2016 1 commit
  21. 20 Apr, 2016 6 commits
  22. 19 Apr, 2016 1 commit