New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TSynSQLSyn.TableNames slow when adding new strings #28

Closed
ansgarbecker opened this Issue May 10, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@ansgarbecker

ansgarbecker commented May 10, 2016

Users of HeidiSQL are experiencing slowness when HeidiSQL adds more than ~1000 strings to a TSynSQLSyn.TableNames. On my Core I7 4500U CPU, 1000 strings need roughly 1 second. A normal TStringList would work much faster, but SynEdit does something in the back with this list, which makes it so slow.

It also does not seem to have an effect if I wrap that in a BeginUpdate/EndUpdate block.

See the latest report from a user: http://www.heidisql.com/forum.php?t=18945#p21272

@Lajos-Juhasz

This comment has been minimized.

Show comment
Hide comment
@Lajos-Juhasz

Lajos-Juhasz Jun 2, 2016

I noticed this bug yesterday. My solution was to build the list in the temporary stringlist and assign the complete list to the highlighter. Most probably the reason for the slowdown is the TSynSQLSyn.TableNamesChanged.

Maybe we should add BeginTableNamesUpdate/EndTableNameUpdates.

Lajos-Juhasz commented Jun 2, 2016

I noticed this bug yesterday. My solution was to build the list in the temporary stringlist and assign the complete list to the highlighter. Most probably the reason for the slowdown is the TSynSQLSyn.TableNamesChanged.

Maybe we should add BeginTableNamesUpdate/EndTableNameUpdates.

@ansgarbecker

This comment has been minimized.

Show comment
Hide comment
@ansgarbecker

ansgarbecker Jun 8, 2016

I already wrapped the TableNames assignment in a Begin/EndUpdate block. Still, one update of that list is still slow.

I did some debugging and found out that it is TSynSQLSyn.PutTableNamesInKeywordList which is slow. Took ~15 seconds for 10,000 table names. To verify that I'm only once in PutTableNamesInKeywordList() I added a showmessage() there, which showed up once, so I'm very sure it's executed one time, not one time per item.

Only, in PutTableNamesInKeywordList() I see some HashKey logic of which know nothing about. Would be nice if someone could speed that procedure up for large amounts of table names.

ansgarbecker commented Jun 8, 2016

I already wrapped the TableNames assignment in a Begin/EndUpdate block. Still, one update of that list is still slow.

I did some debugging and found out that it is TSynSQLSyn.PutTableNamesInKeywordList which is slow. Took ~15 seconds for 10,000 table names. To verify that I'm only once in PutTableNamesInKeywordList() I added a showmessage() there, which showed up once, so I'm very sure it's executed one time, not one time per item.

Only, in PutTableNamesInKeywordList() I see some HashKey logic of which know nothing about. Would be nice if someone could speed that procedure up for large amounts of table names.

uschuster added a commit that referenced this issue Aug 14, 2016

Fix for #28: Use TDictionary<Key, Value> for D2009 and newer to incre…
…ase the performance for adding a lot of table names

the used hash table with a 8-bit bucket size seems not designed for thousands of entries
@uschuster

This comment has been minimized.

Show comment
Hide comment
@uschuster

uschuster Aug 14, 2016

Contributor

The used hash table with a 8-bit bucket size seems not designed for thousands of entries. I have committed a fix that uses TDictionary<Key, Value> for D2009 and newer to increase the performance.

Contributor

uschuster commented Aug 14, 2016

The used hash table with a 8-bit bucket size seems not designed for thousands of entries. I have committed a fix that uses TDictionary<Key, Value> for D2009 and newer to increase the performance.

@ansgarbecker

This comment has been minimized.

Show comment
Hide comment
@ansgarbecker

ansgarbecker Aug 15, 2016

Looks very promising! Thanks so far.
I just merged the updated code in HeidiSQL so I get feedback from relevant users.

ansgarbecker commented Aug 15, 2016

Looks very promising! Thanks so far.
I just merged the updated code in HeidiSQL so I get feedback from relevant users.

@ansgarbecker

This comment has been minimized.

Show comment
Hide comment
@ansgarbecker

ansgarbecker Aug 16, 2016

Got positive feedback (along with more negative comments about totally unrelated stuff) here and here

Thanks a lot for fixing this. Was a long-time bug in Heidi, before I reported that here.

ansgarbecker commented Aug 16, 2016

Got positive feedback (along with more negative comments about totally unrelated stuff) here and here

Thanks a lot for fixing this. Was a long-time bug in Heidi, before I reported that here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment