-1
\$\begingroup\$

I have Two tables Circuit and Tag,

Circuit 
------------------
Id    Name  Path 

Tag
-----------
Id Name Circuit_Id

Now I have to loop through all the tags and update each tag where there is a matching path and name (circuit path and Tag name).

I am doing this and it works fine , but it's really slow. Is there a way I could improve the speed.

foreach (var tag in allPDUTags)
{
    var removedUnderScoreLast = tag.Path.Remove(tag.Path.LastIndexOf('_'));
    var removedUnderScoreLastButOne = removedUnderScoreLast.Remove(removedUnderScoreLast.LastIndexOf('_'));

    var circuitId = _context.Circuits.Where(c => c.Name == removedUnderScoreLastButOne).FirstOrDefault().Id;

    tag.Circuit_Id = circuitId;


    _counter++;

    if ((_counter % 1000) == 0)
    {
        Console.WriteLine($"Updated {_counter} of {totalsTags} Tags");
    }


}
\$\endgroup\$

closed as off-topic by t3chb0t, Raystafarian, Sᴀᴍ Onᴇᴌᴀ, Dannnno, Stephen Rauch Mar 23 '18 at 14:42

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – t3chb0t, Raystafarian, Sᴀᴍ Onᴇᴌᴀ, Dannnno, Stephen Rauch
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Have you posted edited code? There are so many empty lines that it looks like it wasn't your real code. \$\endgroup\$ – t3chb0t Mar 22 '18 at 7:56
  • \$\begingroup\$ @t3chb0t, the C# code is the actual code, SQL table us just the column names \$\endgroup\$ – Simsons Mar 22 '18 at 11:11
  • \$\begingroup\$ What is allPDUTags? Please show the code that creates it. \$\endgroup\$ – Gert Arnold Mar 22 '18 at 16:28
  • 2
    \$\begingroup\$ I'd certainly use EF's capability to build a SQL script and run it directly against the DB. Then you can write a single, optimal query and offload everything to the DB, which should be able to do this in milliseconds. If it can't, add an index to a relevant column or two-- (probably Path and Name). \$\endgroup\$ – Hosch250 Mar 22 '18 at 20:27
  • 1
    \$\begingroup\$ Yes there is a way to improve speed. Use TSQL and do this as a set based operation. \$\endgroup\$ – paparazzo Mar 23 '18 at 11:56
1
\$\begingroup\$
var circuitId = _context.Circuits.Where(c => c.Name == removedUnderScoreLastButOne).FirstOrDefault().Id;

This line is effectively a join inside of your loop meaning that you are going to query the database for every tag in your list. So you have 40k extra queries, which seems likely to be the biggest cause of slowness.

I would focus on moving that join out of your loop. There are a couple of ways you can do that.

  1. Change the allPDUTags to be a the result of a join between Circuit and Tag.
  2. If you can drop out of entity framework writing SQL to update the rows in an efficient manner should be relatively simple given they are both in database tables. I am unsure of the DBMS you are using so I can't provide sample code for that.
  3. Quick and dirty would be to put the list of circuits in memory. Calling _context.Circuits.ToList() would put them all in memory and then compare against that list in your loop. Assumes that circuits is small enough to fit into memory. This option won't scale well and I wouldn't really recommend it.

I would look closely at why Circuit.Path and tag.Name need to be synced in the first place. There might be a historical reason or they might be in 2 different databases/systems but if they are in the same application I would strongly consider normalizing the database and or adding constraints to avoid having to sync in the first place.

\$\endgroup\$
  • \$\begingroup\$ Actually, EF is smart enough to turn that into the following SQL query select top 1 from Foo f where f.Whatever = 'whatever'. \$\endgroup\$ – Hosch250 Mar 22 '18 at 20:25
  • \$\begingroup\$ Yes, _context.Circuits takes around 5 mins amount of time and so does the savechanges method! \$\endgroup\$ – Simsons Mar 23 '18 at 3:40
  • 1
    \$\begingroup\$ @Simsons Now you also talk about SaveChanges. You're not showing all code necessary to do any decent review. See also my comment on your question. \$\endgroup\$ – Gert Arnold Mar 23 '18 at 7:02
  • \$\begingroup\$ @GertArnold so I was right about edited code... \$\endgroup\$ – t3chb0t Mar 23 '18 at 7:22
  • \$\begingroup\$ @Hosch250 Yes but unless I am misunderstanding your comment and EF that command is being run ~40k times. Which of course is going to be slow. It's still effectively a join, just instead of doing a join on all of the data we do it one item at a time in a loop. \$\endgroup\$ – stoj Mar 23 '18 at 15:30
0
\$\begingroup\$

This is a TSQL approach. It should be pretty fast.

declare @Circuit table (id int identity primary key, name varchar(20));
declare @Tag table     (id int identity primary key, path varchar(40), circuitID int);
insert into @tag (path) values ('asld_aslkjf_alskd'), ('asxx_askjf_alsyd_ lasdj');
insert into @Circuit (name) values ('asld'), ('asxx_askjf');

select * from @tag;

update t 
set t.circuitID = c.id 
from @tag t 
join @Circuit c
  on c.name = reverse(SUBSTRING(REVERSE(t.path), CHARINDEX('_', REVERSE(t.path), CHARINDEX('_', REVERSE(t.path)) + 1) + 1, 100))
where t.circuitID <> c.id or t.circuitID is null;

select * from @tag
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.