GitLab FOSS merge requestshttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests2016-10-14T20:07:37Zhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4639Instrument private methods and instance private methods2016-10-14T20:07:37ZPaco GuzmanInstrument private methods and instance private methods## What does this MR do?
Instrument private methods and instance methods by default not just public methods
## Why was this MR needed?
To have more visibility of the performance of the code
## What are the relevant issue numbers?
#1...## What does this MR do?
Instrument private methods and instance methods by default not just public methods
## Why was this MR needed?
To have more visibility of the performance of the code
## What are the relevant issue numbers?
#18527
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- ~~[ ] API support added~~
- [x] Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9yorickpeterse-stagingyorickpeterse-staginghttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4640Measure CPU time for instrumented methods2016-09-15T15:51:01ZPaco GuzmanMeasure CPU time for instrumented methods## What does this MR do?
Measure CPU time for instrumented methods
## Why was this MR needed?
To have more visibility on resource utilisation
## What are the relevant issue numbers?
#18528
## Does this MR meet the acceptance crite...## What does this MR do?
Measure CPU time for instrumented methods
## Why was this MR needed?
To have more visibility on resource utilisation
## What are the relevant issue numbers?
#18528
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- ~~[ ] API support added~~
- [x] Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9yorickpeterse-stagingyorickpeterse-staginghttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4647Include user relationship when retrieving award_emoji2016-10-06T15:34:21ZPaco GuzmanInclude user relationship when retrieving award_emoji## What does this MR do?
Reduce timings in the grouped_awards methods or in how we're showing grouped awards later on, avoiding N+1 queries to the users table
## Are there points in the code the reviewer needs to double check?
I've de...## What does this MR do?
Reduce timings in the grouped_awards methods or in how we're showing grouped awards later on, avoiding N+1 queries to the users table
## Are there points in the code the reviewer needs to double check?
I've decide to load only the award attributes we need `name` and `user_id` that's avoid retrieve only database data that we're going to need and later AR instance doesn't need to populate all their attributes. Would be great if in the includes declaration we can define which attribute we expect for the user objects because the code only needs the user name. If we have something like that maybe could help to speed this up.
This method is used in the following [template](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/views/award_emoji/_awards_block.html.haml#L1-4) and there you can see we call `award_user_list`which will print out the user name, that's where the N+1 query happens
## What are the relevant issue numbers?
Fixes #14320
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- ~~[ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- ~~[ ] API support added~~
- [x] Tests
- ~~[ ] Added for this feature/bug~~
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9yorickpeterse-stagingyorickpeterse-staginghttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4649Resolve "Track the number of new Redis connections per transaction"2016-09-28T22:17:38Zusername-removed-443319Resolve "Track the number of new Redis connections per transaction"## What does this MR do?
Add a new metric counter, `new_redis_connections`, that contains the number of calls to `Redis::Client#connect` in the current transaction.
## Are there points in the code the reviewer needs to double check...## What does this MR do?
Add a new metric counter, `new_redis_connections`, that contains the number of calls to `Redis::Client#connect` in the current transaction.
## Are there points in the code the reviewer needs to double check?
Not sure. I tested this in kind of a brute-force way:
1. Add a debugger in the monkey-patched `connect` method.
2. With metrics enabled, start the app and load a page.
3. The first Redis connection is created by `Rack::Attack` and isn't in a transaction, but still works fine.
4. The second Redis connection is within a transaction (the page load), and increments the counter.
5. If I reload the page, neither debugger is hit.
6. If I use a Redis client and do `CLIENT KILL` on my two existing clients, then reload the page, I get 3 and 4 again.
7. If I disable metrics collection, the debugger never gets hit.
## Why was this MR needed?
We may have a Redis connection leak somewhere, so adding metrics will let us track this.
## What are the relevant issue numbers?
Closes #18451.
## Screenshots (if relevant)
Hahaha nope, not relevant.
## Does this MR meet the acceptance criteria?
- [ ] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- [ ] Tests
- [ ] Added for this feature/bug
- [ ] All builds are passing
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
cc @yorickpeterse 8.9Douwe MaanDouwe Maanhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4674Set inverse_of for Project/Services relation2016-10-09T14:40:37Zyorickpeterse-stagingSet inverse_of for Project/Services relation## What does this MR do?
This MR adds the `inverse_of:` option to two associations to reduce the number of queries when running code such as ` project.gitlab_issue_tracker_service.project`.
## Are there points in the code the reviewer ...## What does this MR do?
This MR adds the `inverse_of:` option to two associations to reduce the number of queries when running code such as ` project.gitlab_issue_tracker_service.project`.
## Are there points in the code the reviewer needs to double check?
No.
## Why was this MR needed?
In !4410 it was revealed code such as the above is used and would run SQL queries when the root object (usually a Project) was already present. By using `inverse_of` Rails can just re-use those Project instances.
## What are the relevant issue numbers?
None.
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] ~~[Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- [x] ~~API support added~~
- [x] Tests
- [x] ~~Added for this feature/bug~~
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9username-removed-128633username-removed-128633https://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4675Eager load project relations in IssueParser2016-10-09T14:40:35Zyorickpeterse-stagingEager load project relations in IssueParser## What does this MR do?
This changes the ReferenceParser class to eager load various associations. This in turn results in the permissions checking code (e.g. the `Ability` model) to _not_ run dozens if not hundreds of extra SQL quer...## What does this MR do?
This changes the ReferenceParser class to eager load various associations. This in turn results in the permissions checking code (e.g. the `Ability` model) to _not_ run dozens if not hundreds of extra SQL queries depending on the amount of references involved (in a single document).
## Are there points in the code the reviewer needs to double check?
No.
## Why was this MR needed?
In !4410 it was revealed a _lot_ of a queries came from the `Ability` model and the code it would call. In many cases this was because the code would simply get a project, then get the owners; or get a group, then get some association of that. Eager loading these associations is a fairly simple solution and greatly cuts down the number of queries.
## What are the relevant issue numbers?
None.
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] ~~[Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- [x] ~~API support added~~
- [ ] Tests
- [x] ~~Added for this feature/bug~~
- [ ] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9Douwe MaanDouwe Maanhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4676Turn Group#owners into a has_many association2016-10-25T02:13:11Zyorickpeterse-stagingTurn Group#owners into a has_many association## What does this MR do?
This turns the regular method `Group#owners` into a `has_many` association.
## Are there points in the code the reviewer needs to double check?
As far as I can tell there's no way to do this without using an i...## What does this MR do?
This turns the regular method `Group#owners` into a `has_many` association.
## Are there points in the code the reviewer needs to double check?
As far as I can tell there's no way to do this without using an intermediate association, but perhaps I'm missing something. The reason an intermediate association is needed is because the supplied Proc is applied to the _final_ association (the one returning users), this means that when using a single `has_many` you can't filter out any intermediate rows (e.g. group members).
## Why was this MR needed?
This code being a regular method would prevent eager loading of the owners of a Group, turning it into a `has_many` association resolves this problem. This was discovered in !4410.
## What are the relevant issue numbers?
None.
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] ~~[Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- [x] ~~API support added~~
- [ ] Tests
- [x] Added for this feature/bug
- [ ] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9username-removed-128633username-removed-128633https://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4702Banzai::Filter::ExternalLinkFilter use XPath2016-10-29T11:38:55ZPaco GuzmanBanzai::Filter::ExternalLinkFilter use XPath## What does this MR do?
Use XPath expresions to search in html documents instead CSS ones
## Why was this MR needed?
To try to speed up the Banzai Filter
## What are the relevant issue numbers?
Closes #18582
## Does th...## What does this MR do?
Use XPath expresions to search in html documents instead CSS ones
## Why was this MR needed?
To try to speed up the Banzai Filter
## What are the relevant issue numbers?
Closes #18582
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9yorickpeterse-stagingyorickpeterse-staginghttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4711Use Git cached counters on project show page2016-11-20T11:08:31ZPaco GuzmanUse Git cached counters on project show page## What does this MR do?
Use Git cached counters for tags and branches to be shown in the project page. Cache this counters any time we build the cache so they are just there when the user visit the page
## What are the relevant issue ...## What does this MR do?
Use Git cached counters for tags and branches to be shown in the project page. Cache this counters any time we build the cache so they are just there when the user visit the page
## What are the relevant issue numbers?
#18709
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- ~~[ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- ~~[ ] API support added~~
- [x] Tests
- ~~[ ] Added for this feature/bug~~
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9yorickpeterse-stagingyorickpeterse-staginghttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4731Remove explicit Gitlab::Metrics.action assignments, are already automatic.2016-11-29T21:26:40ZPaco GuzmanRemove explicit Gitlab::Metrics.action assignments, are already automatic.## What does this MR do?
Remove explicit call not needed anymore
## Are there points in the code the reviewer needs to double check?
NO
## Why was this MR needed?
Remove technical debt
## What are the relevant issue num...## What does this MR do?
Remove explicit call not needed anymore
## Are there points in the code the reviewer needs to double check?
NO
## Why was this MR needed?
Remove technical debt
## What are the relevant issue numbers?
Closes #187608.9yorickpeterse-stagingyorickpeterse-staginghttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4748Filter out sensitive parameters of metrics data2016-11-27T20:50:57ZPaco GuzmanFilter out sensitive parameters of metrics data## What does this MR do?
Filter sensitive parameters
## What are the relevant issue numbers?
Closes https://gitlab.com/gitlab-com/infrastructure/issues/8
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab...## What does this MR do?
Filter sensitive parameters
## What are the relevant issue numbers?
Closes https://gitlab.com/gitlab-com/infrastructure/issues/8
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- ~~[ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- ~~[ ] API support added~~
- [x] Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9yorickpeterse-stagingyorickpeterse-staginghttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4754Track method call times/counts as a single metric2016-11-20T11:08:31Zyorickpeterse-stagingTrack method call times/counts as a single metric## What does this MR do?
This changes method call tracking so only a single metric is emitted regardless of the number of calls. This allows us to more accurately measure the total execution time of a method as well as the number of t...## What does this MR do?
This changes method call tracking so only a single metric is emitted regardless of the number of calls. This allows us to more accurately measure the total execution time of a method as well as the number of times a method is called. See 851e3ff7578973c2206628424eac3b951a3c656d for more details.
## Are there points in the code the reviewer needs to double check?
No.
## Why was this MR needed?
Method call tracking tracked calls individually meaning the end statistics may not always be accurate enough to get a good understanding of where time is spent.
## What are the relevant issue numbers?
None.
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] ~~API support added~~
- [ ] Tests
- [x] Added for this feature/bug
- [ ] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9Robert SpeicherRobert Speicherhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4776Faster diffs2019-09-19T20:53:16ZJacob SchatzFaster diffs## What does this MR do?
Remove the comment buttons on diffs for MR.
## Are there points in the code the reviewer needs to double check?
## Why was this MR needed?
## What are the relevant issue numbers?
## Screenshots (if relevant)
...## What does this MR do?
Remove the comment buttons on diffs for MR.
## Are there points in the code the reviewer needs to double check?
## Why was this MR needed?
## What are the relevant issue numbers?
## Screenshots (if relevant)
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- [ ] Tests
- [ ] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.10Jacob SchatzJacob Schatzhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4787Remove Duplicated keys adding UNIQUE index to fingerprint2016-10-06T15:35:40ZPaco GuzmanRemove Duplicated keys adding UNIQUE index to fingerprint## What does this MR do?
Add a unique index to the keys fingerprint column. We added three migrations:
- Remove duplicates we kept the last key.
- We remove the existing index only when exists (gitlab-ee)
- Add the unique index f...## What does this MR do?
Add a unique index to the keys fingerprint column. We added three migrations:
- Remove duplicates we kept the last key.
- We remove the existing index only when exists (gitlab-ee)
- Add the unique index for gitlab-ce and gitlab-ee
## Why was this MR needed?
To remove a slow query
## What are the relevant issue numbers?
#18697
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- ~~[ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- ~~[ ] API support added~~
- [x] Tests
- ~~[ ] Added for this feature/bug~~
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9yorickpeterse-stagingyorickpeterse-staginghttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4802Remove calls to Rugged::BranchCollection#each from extracts_path before_action2016-11-29T21:26:40ZPaco GuzmanRemove calls to Rugged::BranchCollection#each from extracts_path before_action## What does this MR do?
Check for git data first on the cache later on git.
## Why was this MR needed?
Remove Git data noise on dashboard to keep focusing on reducing those calls. This MR does not close original issue but I think is ...## What does this MR do?
Check for git data first on the cache later on git.
## Why was this MR needed?
Remove Git data noise on dashboard to keep focusing on reducing those calls. This MR does not close original issue but I think is going to give us a better insight in what is happening.
## What are the relevant issue numbers?
#18709
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- ~~[ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- ~~[ ] API support added~~
- [x] Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9yorickpeterse-stagingyorickpeterse-staginghttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4803Cache Participable#participants in instance variable2016-10-25T02:13:32ZPaco GuzmanCache Participable#participants in instance variable## Why was this MR needed?
Avoid repetitive calls to consuming methods
## What are the relevant issue numbers?
Closes #18792
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/bl...## Why was this MR needed?
Avoid repetitive calls to consuming methods
## What are the relevant issue numbers?
Closes #18792
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- ~~[ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- ~~[ ] API support added~~
- [x] Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9yorickpeterse-stagingyorickpeterse-staginghttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4813Optimize Banzai::Filter::RelativeLinkFilter2016-10-28T01:09:16Zusername-removed-367626Optimize Banzai::Filter::RelativeLinkFilter## What does this MR do?
Optimize Banzai::Filter::RelativeLinkFilter
A lot of git operations were being repeated, for example, to build a url
you would ask if the path was a Tree, which would call a recursive routine
in `Gitlab::...## What does this MR do?
Optimize Banzai::Filter::RelativeLinkFilter
A lot of git operations were being repeated, for example, to build a url
you would ask if the path was a Tree, which would call a recursive routine
in `Gitlab::Git::Tree#where`, then ask if the path was a Blob, which would
call a recursive routine at `Gitlab::Git::Blob#find`, making reference to
the same git objects several times. Now we call `Rugged::Tree#path`, which
allows us to determine the type of the path in one pass.
Some other minor improvement added, like saving commonly used references
instead of calculating them each time.
## Are there points in the code the reviewer needs to double check?
No
## Why was this MR needed?
Banzai::Filter::RelativeLinkFilter is snow
## What are the relevant issue numbers?
Closes #18590
## Screenshots (if relevant)
For testing I prepared a file with many relative (to the current and to the root) paths repeated many times. The requests took the following times:
**BEFORE**
![Captura_de_pantalla_2016-06-20_a_las_7.15.27_p.m.](/uploads/2d07bdb8267d2282022734d11175138c/Captura_de_pantalla_2016-06-20_a_las_7.15.27_p.m..png)
**AFTER**
![Captura_de_pantalla_2016-06-20_a_las_7.18.36_p.m.](/uploads/b467e47c598c63e866846960c4a21590/Captura_de_pantalla_2016-06-20_a_las_7.18.36_p.m..png)
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] Tests
- [x] Added for this feature/bug
- [ ] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
/cc @pcarranza 8.9yorickpeterse-stagingyorickpeterse-staginghttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4828Support for rendering/redacting multiple documents2017-03-15T12:31:52Zyorickpeterse-stagingSupport for rendering/redacting multiple documents## What does this MR do?
This MR does two things:
1. Separate redacting HTML documents from the htm-pipeline Gem and support redacting multiple documents at once
2. Add code that supports rendering and redacting multiple Markdown docum...## What does this MR do?
This MR does two things:
1. Separate redacting HTML documents from the htm-pipeline Gem and support redacting multiple documents at once
2. Add code that supports rendering and redacting multiple Markdown documents
See commit d7dc68ef878d2ad032efa92e25a8fa77250f9099 for more information.
## Are there points in the code the reviewer needs to double check?
Yes. The way notes are currently gathered in the controllers is something I'm not very fond of.
## Why was this MR needed?
Most of this is explained in d7dc68ef878d2ad032efa92e25a8fa77250f9099 but in short:
* Redacting happened per document, leading to lots of queries being executed for every comment.
* On GitLab.com between 1 and ~2.5 seconds is spent in redacting documents per request to `Projects::IssuesController#show`
* Redacting multiple documents at once greatly reduces the number of SQL queries executed.
## What are the relevant issue numbers?
#18581
## Screenshots (if relevant)
Before/after of the method timings:
![redact_timings](/uploads/cd13f02f32be3df81deac5aa64eee576/redact_timings.png)
Here the green bars are the timings for the `RedactorFilter#call` method, the orange-ish timings are for `Banzai::Redactor#redact`.
SQL query count impact:
![redact_query_timings](/uploads/a58201285109e5b8f2705b6d02bdf069/redact_query_timings.png)
Here the big drop just before 12:30 is when I switched to the branch of this MR, the number of queries dropped from around 4000 to around 2600.
## Does this MR meet the acceptance criteria?
- [ ] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] ~~[Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- [x] ~~API support added~~
- Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9yorickpeterse-stagingyorickpeterse-staginghttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4830Move pre_process into render_result2016-11-27T20:50:59Zyorickpeterse-stagingMove pre_process into render_result## What does this MR do?
This MR moves `Banzai::Renderer.pre_process` into `Banzai::Renderer.render_result`.
## Are there points in the code the reviewer needs to double check?
No.
## Why was this MR needed?
The `pre_proce...## What does this MR do?
This MR moves `Banzai::Renderer.pre_process` into `Banzai::Renderer.render_result`.
## Are there points in the code the reviewer needs to double check?
No.
## Why was this MR needed?
The `pre_process` method was called even when its output would be ignored. See 11a5a4f359ee57029dbfcc9185fc6b47243ea2aa for more details.
## What are the relevant issue numbers?
None
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] ~~[Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- [x] ~~API support added~~
- Tests
- [x] Added for this feature/bug
- [ ] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9Robert SpeicherRobert Speicherhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/4859Use memorized tags array when searching tags by name2016-09-27T14:29:24Zusername-removed-367626Use memorized tags array when searching tags by name## What does this MR do?
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4423 introduced a sorting dropdown for the tags page. As a side effect, the paginated array used is now of Tag objects instead of tag name Strings, and th...## What does this MR do?
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4423 introduced a sorting dropdown for the tags page. As a side effect, the paginated array used is now of Tag objects instead of tag name Strings, and therefore the name sorting (the default) does:
````diff
+ VersionSorter.rsort(tag_names).map { |tag_name| find_tag(tag_name) }
````
The `find_tag` method is non-memorized, so it hits the repository for every tag. This MR updates it to use the memorized tags array
## Are there points in the code the reviewer needs to double check?
No
## Why was this MR needed?
https://gitlab.com/gitlab-org/gitlab-ce/tags is not loading
## What are the relevant issue numbers?
Closes #18924
## Screenshots (if relevant)
On my local machine:
*BEFORE*
![Captura_de_pantalla_2016-06-22_a_las_11.22.26_a.m.](/uploads/c46d693c43f47f49739ea590abd61890/Captura_de_pantalla_2016-06-22_a_las_11.22.26_a.m..png)
![Captura_de_pantalla_2016-06-22_a_las_11.22.11_a.m.](/uploads/eb4c51d92cf5a0cc4902b712778f8779/Captura_de_pantalla_2016-06-22_a_las_11.22.11_a.m..png)
*AFTER*
![Captura_de_pantalla_2016-06-22_a_las_11.21.16_a.m.](/uploads/1a02a57800876cb583242ea415a66aa4/Captura_de_pantalla_2016-06-22_a_las_11.21.16_a.m..png)
![Captura_de_pantalla_2016-06-22_a_las_11.21.27_a.m.](/uploads/e21adb664c8532cec4da368fd9531a50/Captura_de_pantalla_2016-06-22_a_las_11.21.27_a.m..png)
## Does this MR meet the acceptance criteria?
- [ ] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)8.9yorickpeterse-stagingyorickpeterse-staging