Skip to content
Snippets Groups Projects

Execute specific named route method from toggle_award_url helper method

Merged Paco Guzman requested to merge faster_toggle_award_url_helper_method into master
All threads resolved!

What does this MR do?

Use specific named route method because this helper method is called a lot of times in pages with a lot of discussions.

Are there points in the code the reviewer needs to double check?

Why was this MR needed?

Screenshots (if relevant)

require 'benchmark/ips'

project = Project.find_with_namespace("gitlab-org/gitlab-ce")
project.namespace # Association load
namespace = project.namespace.becomes(Namespace)
note = project.notes.last

Benchmark.ips do |x|
  x.report("current") do
    app.url_for([:toggle_award_emoji, project.namespace.becomes(Namespace), project, note])
  end

  x.report("inter1") do
    app.url_for([:toggle_award_emoji, namespace, project, note])
  end

  x.report("inter2") do
    app.toggle_award_emoji_namespace_project_note_url(namespace, project, note)
  end

  x.report("new") do
    app.toggle_award_emoji_namespace_project_note_url(namespace_id: project.namespace_id, project_id: project.id, id: note.id)
  end

  x.compare!
end
Calculating -------------------------------------
             current   164.000  i/100ms
              inter1   358.000  i/100ms
              inter2   749.000  i/100ms
                 new   956.000  i/100ms
-------------------------------------------------
             current      1.625k (± 8.1%) i/s -      8.200k
              inter1      3.657k (± 6.3%) i/s -     18.258k
              inter2      7.784k (± 8.3%) i/s -     38.948k
                 new      9.821k (± 9.3%) i/s -     48.756k

Comparison:
                 new:     9821.4 i/s
              inter2:     7783.6 i/s - 1.26x slower
              inter1:     3656.5 i/s - 2.69x slower
             current:     1624.6 i/s - 6.05x slower

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Relates with #23241 (moved) and #23226 (moved)

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Yorick Peterse Added ~18308 label

    Added ~18308 label

  • Reassigned to @pacoguzman

  • Paco Guzman Resolved all discussions

    Resolved all discussions

  • Paco Guzman Added 151 commits:

    Added 151 commits:

    • 72e8c435...602cac52 - 150 commits from branch master
    • 4adab57e - Execute specific named route method from toggle_award_url helper method

    Compare with previous version

  • Reassigned to @pacoguzman

  • Yorick Peterse Resolved all discussions

    Resolved all discussions

  • Paco Guzman Added 90 commits:

    Added 90 commits:

    • 4adab57e...5c9a54d6 - 89 commits from branch master
    • 0534a43d - Execute specific named route method from toggle_award_url helper method

    Compare with previous version

  • Paco Guzman Added 8 commits:

    Added 8 commits:

    • 0534a43d...77507df6 - 7 commits from branch master
    • 6c7a6aa1 - Execute specific named route method from toggle_award_url helper method

    Compare with previous version

  • Paco Guzman Added 59 commits:

    Added 59 commits:

    • 6c7a6aa1...4e6af0c3 - 58 commits from branch master
    • 6a16697a - Execute specific named route method from toggle_award_url helper method

    Compare with previous version

  • Author Contributor

    @yorickpeterse finally passed the build after multiple rebases against master. The warning in the build is about merging this into ee, I've tried to follow the instructions but it will rebase rebase 326 commits (probably the difference between current gitlab-ce and gitlab-ee) I don't think that rake is correct, I've checked the files in both repositories and they have the same content so I think won't be a conflict on that merge into gitlab-ee

  • Yorick Peterse Added ~149423 label

    Added ~149423 label

  • Yorick Peterse Marked the task CHANGELOG entry added as completed

    Marked the task CHANGELOG entry added as completed

  • Marked the task Documentation created/updated as completed

  • Yorick Peterse Marked the task API support added as completed

    Marked the task API support added as completed

  • Yorick Peterse Marked the task Added for this feature/bug as completed

    Marked the task Added for this feature/bug as completed

  • Yorick Peterse Marked the task All builds are passing as completed

    Marked the task All builds are passing as completed

  • Yorick Peterse Marked the task Conform by the merge request performance guides as completed

    Marked the task Conform by the merge request performance guides as completed

  • Yorick Peterse Marked the task Conform by the style guides as completed

    Marked the task Conform by the style guides as completed

  • Yorick Peterse Marked the task Branch has no merge conflicts with master (if it does - rebase it please) as completed

    Marked the task Branch has no merge conflicts with master (if it does - rebase it please) as completed

  • Yorick Peterse Marked the task Conform by the merge request performance guides as incomplete

    Marked the task Conform by the merge request performance guides as incomplete

  • Yorick Peterse Marked the task Conform by the style guides as incomplete

    Marked the task Conform by the style guides as incomplete

  • Yorick Peterse Marked the task Branch has no merge conflicts with master (if it does - rebase it please) as incomplete

    Marked the task Branch has no merge conflicts with master (if it does - rebase it please) as incomplete

  • Yorick Peterse Marked the task Conform by the merge request performance guides as completed

    Marked the task Conform by the merge request performance guides as completed

  • Yorick Peterse Marked the task Conform by the style guides as completed

    Marked the task Conform by the style guides as completed

  • Yorick Peterse Marked the task Branch has no merge conflicts with master (if it does - rebase it please) as completed

    Marked the task Branch has no merge conflicts with master (if it does - rebase it please) as completed

  • Marked the task Squashed related commits together as completed

  • Yorick Peterse Marked the task Branch has no merge conflicts with master (if it does - rebase it please) as incomplete

    Marked the task Branch has no merge conflicts with master (if it does - rebase it please) as incomplete

  • Marked the task Squashed related commits together as incomplete

  • Yorick Peterse Marked the task Added for this feature/bug as incomplete

    Marked the task Added for this feature/bug as incomplete

  • Yorick Peterse Marked the task All builds are passing as incomplete

    Marked the task All builds are passing as incomplete

  • Yorick Peterse Marked the task Conform by the merge request performance guides as incomplete

    Marked the task Conform by the merge request performance guides as incomplete

  • Yorick Peterse Marked the task Conform by the style guides as incomplete

    Marked the task Conform by the style guides as incomplete

  • Yorick Peterse Marked the task Added for this feature/bug as completed

    Marked the task Added for this feature/bug as completed

  • Yorick Peterse Marked the task All builds are passing as completed

    Marked the task All builds are passing as completed

  • Yorick Peterse Marked the task Conform by the merge request performance guides as completed

    Marked the task Conform by the merge request performance guides as completed

  • Yorick Peterse Status changed to merged

    Status changed to merged

  • Yorick Peterse Mentioned in commit b1be3220

    Mentioned in commit b1be3220

  • Mentioned in issue #23506 (closed)

  • Already in 8.13.0-rc3.

  • Rémy Coutable Removed ~149423 label

    Removed ~149423 label

  • Paco Guzman Mentioned in merge request !6967 (merged)

    Mentioned in merge request !6967 (merged)

  • Please register or sign in to reply
    Loading