Speed up code

GrooverFromHolland

Hi all, I know that I write crappy code, but the code snippet below takes 16 seconds on my machine in debug and that is unacceptable. Please help me to speed it up.

string valueString = string.Empty;
//List<byte> AtronBytes First 39 bytes = header
int length = AtronBytes.Count - 39;
for (int i = 0; i < length; i += 8) // length = 1176
{
int colNum;
if (i == 0) colNum = 1;
else colNum = i / 8 + 1;
string column = machineDataColumnNames[colNum];
//tempAtronBytes = copy of AtronBytes
byte[] valueByte = tempAtronBytes.GetRange(0, 8).ToArray();
// last byte of valueByte = checksum
byte chkVal = Extentions.CalculateOpdrChecksum(tempAtronBytes.GetRange(0, 7).ToArray()); // 1ms
if (chkVal == tempAtronBytes[7])// checksum test
{
valueString = Encoding.ASCII.GetString(
tempAtronBytes.GetRange(0, 7).ToArray()).Trim();
// Remove value bytes and checksum:
tempAtronBytes.RemoveRange(0, 8);

              SqlCeExtentions.ZakdataDatabase\_Update(column, valueString, opdrString); // 87ms
              // add to stringBuilder:
              logAppend(column.PadRight(30), valueString);
              //Show in richTextbox
              if (colNum > 146)
              {
                  logResult();
              }
          }
          else
          {
              MessageBox.Show("Checksum error");
              return;
          }
      }

Thanks, Groover

0200 A9 23 0202 8D 01 80 0205 00

OriginalGriff

You need to look closely at your numbers, and at the other code involved. You say the loop runs for 1107 iterations, and that ZakdataDatabase_Updatetakes 87ms - so if your code called the method each time round, then it wouldn't be 16 seconds, it would be 1107 * 0.087 seconds: 96 seconds. So the chances are that it isn't calling it every time, but that the slowdown is in that method. At a guess, it might be worth trying to build up a table of updates, and carrying them out as a single bulk operation, or at the very least looking at what that method does and how it does it to see if you can bring that time down. [edit]Gah! I got the numbers wrong (I missed the "i += 8") which reduces it to 12 secondsStill the same problem and solution, though![/edit]

Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...

Sascha Lefevre

for (int i = 0; i < length; i += 8) // length = 1176

-> 1176 / 8 = 147 loop executions

SqlCeExtentions.ZakdataDatabase_Update(column, valueString, opdrString); // 87ms

-> 147 * 87ms = 12.8 seconds

the code snippet below takes 16 seconds

-> 12.8s * 100% / 16s = 80% The database call makes up for 80% of the time spend for that code. You don't need to optimize the code that you've shown here, you need to optimize the database call. You probably should build a batch statement and use that to update the database only once after the loop is done.

If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson

OriginalGriff

:doh: I missed the "i += 8" - your numbers are right, mine are wrong. But we both think it's the same thing, which is good. You get my upvote and I've edited mine!

Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...

GrooverFromHolland

Thank You for your fast response. I should have done the math myself :doh:! Unfortunately there is no bulk-copy for SQL Compact Edition, so the only way I think Is to show the data first in the richTextBox and insert it in the database on a different thread and notify the user when it is successfully finished. Or is there another way? Groover,

0200 A9 23 0202 8D 01 80 0205 00

Sascha Lefevre

To be honest, I didn't spend too much thought on the fact that you're using SqlCe. (If you're planning on using this application for several years you might want to consider using a different database because SqlCe is "in deprecation state", meaning that there won't be any new versions and support is running out soon-ish.) I don't really know about the restrictions of SqlCe compared to SQL Server, but Google seems to tell me that SqlCe does not only not support bulk copy but also doesn't support batch statements, which I was originally thinking about. FYI, in "real" database systems you can execute multiple statements at once by sending them as one command-text ("batch"), separated with a semi-colon, e.g. this could be executed with a single command on SQL Server:

UPDATE myTable SET col1 = @p1 WHERE id = @p2;
UPDATE myTable SET col1 = @p3 WHERE id = @p4;
UPDATE myTable SET col1 = @p5 WHERE id = @p6;

But SqlCe should at least allow you to execute multiple commands (each with one update-statement) within one transaction, which would be the next best solution and should also speed up the process greatly so that you wouldn't have to resort to creating a separate thread for that. I found two projects which might be helpful (though the former suggestion shouldn't be hard to implement): 1) SQL Compact Bulk Insert Library - Home[^] (Apparently only for inserting, not updating) 2) GitHub - lokiworld/Lyare.SqlServerCe.MultiQuery: An extension to the Microsoft SQL Server Compact Edition client library, which simulates the support of the multiple statements in a single command.[^]

If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson

Lost User

I haven't used SQLCE for some time, but the alternative SQLite. SQLite is slow if you update (or insert) in a loop, unless you wrap it in a transaction. I'd also recommend using a List to hold the data, and not a RTF which needs to be redrawn on each modification.

Bastard Programmer from Hell :suss: If you can't read my code, try converting it here[^][](X-Clacks-Overhead: GNU Terry Pratchett)

Lost User

You should "count" the number of database inserts / accesses; I think it makes it easier to contemplate the thing. For example, using Entity Framework and SQL Server Express, I can sustain over 1000 table inserts per second while loading entire object graphs.

Patrice T

Only a detail replace

      for (int i = 0; i < length; i += 8) // length = 1176
      {
          int colNum;
          if (i == 0) colNum = 1;
          else colNum = i / 8 + 1;

with

      int colNum=0;
      for (int i = 0; i < length; i += 8) // length = 1176
      {
          colNum ++;

It will not really change the overall timing, but should improve a little.

Patrice “Everything should be made as simple as possible, but no simpler.” Albert Einstein

GrooverFromHolland

You are absolutely right, I did mention that I write crappy code. :) changed it to:

int colNum = 0;
for (int i = 0; i < length; i += 8)
{
colNum = i / 8 + 1;

and zero divided by 8 gives zero, so no need for checking that. :doh: Thanks, Groover

0200 A9 23 0202 8D 01 80 0205 00

Patrice T

Your change correct, but mine is faster.

Patrice “Everything should be made as simple as possible, but no simpler.” Albert Einstein

GrooverFromHolland

I followed Your advice and went for SQLite database and entity framework EF6 and the total time needed for the same same code is now 1400 ms vs 16 sec before! It took some time to learn and implement EF(hence the late reply) but it was worth the efford :) Thank You, Groover

0200 A9 23 0202 8D 01 80 0205 00

Lost User

Glad it worked out for you! Didn't mean to say you should use EF; just that more "benchmark numbers" would be useful. But yes; EF lets us think a little less about data access strategies and avoid some mistakes that can cause performance problems. If you haven't already, these were good starting links for EF: Entity Framework (EF) Documentation[^]