SqlServerSequenceValueGenerator inefficient

[cherrydev]

Because the value generator opens a connection to the database for every single sequence that it generates, and then ALSO fetches only a single value at a time, it is (even compared to identity columns) excruciatingly slow. The base class for SqlServerSequenceValueGenerator has a BlockSize property, but it's not being used when requesting a new sequence number. It makes sense to me that sp_sequence_get_range should be used instead of "SELECT NEXT VALUE FOR" since it allows for fetching an arbitrary number of values at the same time.

[rowanmiller]

Investigate as this isn't how it is supposed to work - if it is then we need to fix

[lukewaters]

In HiLoValueGeneratorState we use the block size after we call GetNewHighValue()

                        var newCurrent = getNewHighValue();
                        newValue = new HiLoValue(newCurrent, newCurrent + _blockSize);
                        _pool[poolIndexToUse] = newValue;

If the block size is set to 1, we will get a new block every time we request a value, but otherwise it is only called once per block. It appears to me that we should rename GetNewHighValue to GetNewLowValue, as this function actually gets the next available value, then creates a block by adding the block size to that value.

[lukewaters]

the code is here https://github.com/aspnet/EntityFramework/blob/4aafc3b614027cb7bbbe643db3324a96e2893cb4/src/EntityFramework.Core/ValueGeneration/HiLoValueGeneratorState.cs#L51-L53

[lukewaters]

@cherrydev What type of values are you using for your keys when you see this behavior?

[lukewaters]

Closing as no repro, will reopen if more information is posted

[cherrydev]

Sorry I didn't follow up for this. I was just using integer keys:

modelBuilder.ForSqlServer.UseSequence();

With no other explicit configuration.

While looking at the SQL logs, I was most definitely seeing one "NEXT VALUE FOR " for each entity that was being inserted when SaveChanges() was called on the entity. I did not have it configured explicitly for HiLoValueGenerator (nor do I actually know how to do that).

[lukewaters]

the following test demonstrates the behavior of our batched key generator. For the first 5 keys we query the database for a new key block, but then we continue using those same 5 key blocks until we hit 50 entries. Then we repeat.

        [Theory]
        [InlineData(105, 15)]
        [InlineData(101, 11)]
        [InlineData(100, 10)]
        [InlineData(30, 5)]
        [InlineData(20, 5)]
        [InlineData(2, 2)]
        public void Keys_generated_in_batches(int count, int expected)
        {
            var loggerFactory = new TestSqlLoggerFactory();
            var serviceProvider = new ServiceCollection()
                .AddEntityFramework()
                .AddSqlServer()
                .ServiceCollection()
                .AddInstance<ILoggerFactory>(loggerFactory)
                .AddSingleton<SqlServerModificationCommandBatchFactory, TestSqlServerModificationCommandBatchFactory>()
                .BuildServiceProvider();

            using (var context = new ConfiguredChipsContext(serviceProvider, "KettleChips"))
            {
                context.Database.EnsureCreated();

                for (int i = 0; i < count; i++)
                {
                    context.Chips.Add(new KettleChips { BestBuyDate = DateTime.Now, Name = "Doritos Locos Tacos " + i });
                }
                context.SaveChanges();
            }

            Assert.Equal(expected, CountSqlOccurrences("SELECT NEXT VALUE FOR"));
        }

[divega]

@ajcvickers I believe this one is ready to close.