Commit 6ff3f6e9 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Improve finding of terminus when shifting items

We already have the min / max position from the previous query so we
just need to find the object with that position instead of querying and
sorting all objects again and finding the min / max one.

Changelog: performance
parent bafd3ab4
...@@ -53,13 +53,18 @@ module RelativePositioning ...@@ -53,13 +53,18 @@ module RelativePositioning
return [size, starting_from] if size >= MIN_GAP return [size, starting_from] if size >= MIN_GAP
terminus =
if Feature.enabled?(:optimize_shifting_relative_positions, default_enabled: :yaml)
context.at_position(starting_from)
else
at_end ? context.max_sibling : context.min_sibling
end
if at_end if at_end
terminus = context.max_sibling
terminus.shift_left terminus.shift_left
max_relative_position = terminus.relative_position max_relative_position = terminus.relative_position
[[(MAX_POSITION - max_relative_position) / gaps, IDEAL_DISTANCE].min, max_relative_position] [[(MAX_POSITION - max_relative_position) / gaps, IDEAL_DISTANCE].min, max_relative_position]
else else
terminus = context.min_sibling
terminus.shift_right terminus.shift_right
min_relative_position = terminus.relative_position min_relative_position = terminus.relative_position
[[(min_relative_position - MIN_POSITION) / gaps, IDEAL_DISTANCE].min, min_relative_position] [[(min_relative_position - MIN_POSITION) / gaps, IDEAL_DISTANCE].min, min_relative_position]
......
---
title: Improve shifting of positions when creating issues
merge_request: 59745
author:
type: performance
---
name: optimize_shifting_relative_positions
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59745
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/328402
milestone: '13.12'
type: development
group: group::project management
default_enabled: false
...@@ -13,6 +13,7 @@ module Gitlab ...@@ -13,6 +13,7 @@ module Gitlab
MIN_GAP = 2 MIN_GAP = 2
NoSpaceLeft = Class.new(StandardError) NoSpaceLeft = Class.new(StandardError)
InvalidPosition = Class.new(StandardError)
IllegalRange = Class.new(ArgumentError) IllegalRange = Class.new(ArgumentError)
def self.range(lhs, rhs) def self.range(lhs, rhs)
......
...@@ -129,6 +129,14 @@ module Gitlab ...@@ -129,6 +129,14 @@ module Gitlab
neighbour(sib) neighbour(sib)
end end
def at_position(position)
item = scoped_items.find_by(relative_position: position)
raise InvalidPosition, 'No item found at the specified position' if item.nil?
neighbour(item)
end
def shift_left def shift_left
move_sequence_before(true) move_sequence_before(true)
object.reset_relative_position object.reset_relative_position
......
...@@ -212,4 +212,20 @@ RSpec.describe Gitlab::RelativePositioning::ItemContext do ...@@ -212,4 +212,20 @@ RSpec.describe Gitlab::RelativePositioning::ItemContext do
end end
end end
end end
describe '#at_position' do
let_it_be(:issue) { create_issue(500) }
let_it_be(:issue_2) { create_issue(510) }
let(:subject) { described_class.new(issue, range) }
it 'finds the item at the specified position' do
expect(subject.at_position(500)).to eq(described_class.new(issue, range))
expect(subject.at_position(510)).to eq(described_class.new(issue_2, range))
end
it 'raises InvalidPosition when the item cannot be found' do
expect { subject.at_position(501) }.to raise_error Gitlab::RelativePositioning::InvalidPosition
end
end
end end
...@@ -66,7 +66,7 @@ RSpec.shared_examples 'a class that supports relative positioning' do ...@@ -66,7 +66,7 @@ RSpec.shared_examples 'a class that supports relative positioning' do
end end
end end
describe '.move_nulls_to_end' do shared_examples '.move_nulls_to_end' do
let(:item3) { create_item } let(:item3) { create_item }
let(:sibling_query) { item1.class.relative_positioning_query_base(item1) } let(:sibling_query) { item1.class.relative_positioning_query_base(item1) }
...@@ -186,7 +186,7 @@ RSpec.shared_examples 'a class that supports relative positioning' do ...@@ -186,7 +186,7 @@ RSpec.shared_examples 'a class that supports relative positioning' do
end end
end end
describe '.move_nulls_to_start' do shared_examples '.move_nulls_to_start' do
let(:item3) { create_item } let(:item3) { create_item }
let(:sibling_query) { item1.class.relative_positioning_query_base(item1) } let(:sibling_query) { item1.class.relative_positioning_query_base(item1) }
...@@ -261,6 +261,24 @@ RSpec.shared_examples 'a class that supports relative positioning' do ...@@ -261,6 +261,24 @@ RSpec.shared_examples 'a class that supports relative positioning' do
end end
end end
context 'when optimize_shifting_relative_positions is enabled' do
before do
stub_feature_flags(optimize_shifting_relative_positions: true)
end
it_behaves_like '.move_nulls_to_start'
it_behaves_like '.move_nulls_to_end'
end
context 'when optimize_shifting_relative_positions is disabled' do
before do
stub_feature_flags(optimize_shifting_relative_positions: false)
end
it_behaves_like '.move_nulls_to_start'
it_behaves_like '.move_nulls_to_end'
end
describe '#move_before' do describe '#move_before' do
let(:item3) { create(factory, default_params) } let(:item3) { create(factory, default_params) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment