-3

I have the following search in a DataGridView with linq but the search is too slow, how can I optimize this query or why is this happening? there are 800 records inside the DataGridView

private void txtBusqueda_TextChanged(object sender, EventArgs e)
{
    var filtro = (from p in Global.ArticulosGlobales 
                  where p.codigo.ToUpper().StartsWith(txtBusqueda.Text.ToUpper()) 
                  select p).ToList();
    dtGridDatos.DataSource = filtro;
}
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Esteban Giraldo
  • 151
  • 1
  • 1
  • 9
  • https://stackoverflow.com/a/23814658/11683, and don't do it on each text change? – GSerg Jun 18 '21 at 20:52
  • `p.codigo.ToUpper().StartsWith(txtBusqueda.Text.ToUpper()) ` - this is not SARGable. – Dai Jun 18 '21 at 20:56
  • Is `Global.ArticulosGlobales` Linq-to-Entities (Entity Framework) or Linq-to-Objects? – Dai Jun 18 '21 at 20:57
  • is Linq-to-Entities (Entity Framework) – Esteban Giraldo Jun 18 '21 at 21:02
  • 2
    @EstebanGiraldo You should not be using a long-life `DbContext` - so `Global` should not be a field, it should be a local variable that's created and destroyed within a single method's lifetime. This is because `DbContext` represents a _unit-of-work_. – Dai Jun 18 '21 at 21:06

1 Answers1

0

Linq-to-Entities

Using functions on columns in a query renders the predicate non-SARGable (there are exceptions if you have a materialized view or index, but I assume that doesn't apply in this case).

Nevertheless, given that ISO SQL is case-insensitive by default it means you don't need to perform case-transformation at all as all string comparisons in Linq queries will be case-insensitive unless you're using a non-default collation which is explicitly case-sensitive.

Also ensure that you have the codigo column indexed so prefix searches are optimal:

CREATE INDEX IX_codigo ON dbo.ArticulosGlobales ( codigo );

So this will do:

String prefix = this.txtBusqueda.Text;

var filtro = await Global.ArticulosGlobales
    .Where( p => p.codigo.StartsWith( prefix ) )
    .ToListAsync()
    .ConfigureAwait(true); // <-- Explicit continuation on UI thread.

dtGridDatos.DataSource = filtro;

Linq-to-Objects

As with Linq-to-Entities, using ToUpper() is the worst way to perform a case-insensitive string comparison - using it inside a lambda also means excessive string allocations, which is a bad thing. Instead, use StringComparer.OrdinalIgnoreCase (or StringComparison.OrdinalIgnoreCase if it's a parameter).

Also, save this.txtBusqueda.Text to a temporary variable because the .Text accessor on the TextBox is non-trivial and performs thread-access checks, which further slow things down.

String prefix = this.txtBusqueda.Text;

var filtro = Global.ArticulosGlobales
    .Where( p => p.codigo.StartsWith( prefix, StringComparison.OrdinalIgnoreCase ) )
    .ToList()

dtGridDatos.DataSource = filtro;

Further optimization: caching previous results

I assume this is a find-as-you-type filter - in which case be aware that as the user types in a new letter, each subsequent set of results is a subset of the previous result-set, which means you only need to hit the database once and store those results in-memory (and store them in an prefix-search-optimized structure, of course). All subsequent more-specific searches would then operate on the last obtained subset.

An initial optimization, which can still be improved tremendously, might look like this:

private readonly Dictionary< String, List<ArticulosGlobales > > searchSubsets = new Dictionary< String, List<ArticulosGlobales > >( StringComparer.OrdinalIgnoreCase );

private String connectionString = "...";

private async Task LoadAsync()
{
    List<ArticulosGlobales> all;
    using( MyDbContext db = new MyDbContext( this.connectionString ) )
    {
        all = await db.ArticulosGlobales
            .OrderBy( p => p.codigo )
            .ToListAsync()
            .ConfigureAwaita(false);
    }

    this.searchSubsets.Clear();
    this.searchSubsets[ String.Empty ] = all;
}

private async void txtBusqueda_TextChanged(object sender, EventArgs e)
{
    if( this.searchSubsets.Count == 0 )
    {
        await this.LoadAsync();
    }

    String search = ((TextBox)sender).Text;
    
    // Has this search already been performed?
    if( this.searchSubsets.TryGetValue( search, out List<ArticulosGlobales> cachedResults ) )
    {
        dtGridDatos.DataSource = cachedResults;
        return;
    }
    
    // Is there a previous search we can take a subset of?
    for( Int32 i = search.Length - 2; i >= 0; --i ) 
    {
        String prefix = search.Substring( 0, i );
        if( this.searchSubsets.TryGetValue( prefix, out List<ArticulosGlobales> prefixResults )
        {
            List<ArticulosGlobales> subset = prefixResults
                .Where( p => p.codigo.StartsWith( search, StringComparison.OrdinalIgnoreCase ) )
                .ToList();

            this.searchSubsets[ search ] = subset;
            dtGridDatos.DataSource = cachedResults;
            return;
        }
    }

    // Program will never get to this point because the `for` loop above will eventually use an empty-string with `this.searchSubsets.TryGetValue` which *will* have a result, so just throw an exception here instead to keep the C# compiler happy:
    throw new InvalidOperationException( "This will never be thrown." );
}
Dai
  • 141,631
  • 28
  • 261
  • 374