Speed up code
-
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
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...
-
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
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
-
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
: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...
-
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
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
-
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
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
-
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
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)
-
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
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.
-
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
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
-
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
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
-
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
-
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.
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
-
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
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[^]
