Skip to content
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

[PATCH] Performance improvements in fetch of string data [DNET452] #154

Closed
firebird-automations opened this issue Aug 10, 2012 · 9 comments

Comments

@firebird-automations
Copy link

Submitted by: Valeriy K. (darkside)

Attachments:
before.png
after.png

While working on performance improvement in my application, I have found a bottleneck in FirebirdSql.Data.Client.Common.XsqldaMarshaler.GetString(Charset, Byte[]).

After changing:

return value.Replace('\0', ' ').Trim();

To:

return value.TrimEnd('\0', ' ');

I get 4x performance boost.
Please, see attached profiler screenshots.

Commits: 9162dfd

@firebird-automations
Copy link
Author

Modified by: Valeriy K. (darkside)

Attachment: before.png [ 12189 ]

@firebird-automations
Copy link
Author

Modified by: Valeriy K. (darkside)

Attachment: after.png [ 12190 ]

@firebird-automations
Copy link
Author

Modified by: Valeriy K. (darkside)

description: While working on performance improvement in my application, I have founded a bottleneck in FirebirdSql.Data.Client.Common.XsqldaMarshaler.GetString(Charset, Byte[]).

After changing:

return value.Replace('\0', ' ').Trim();

To:

return value.TrimEnd('\0', ' ');

I get 4x performance boost.
Please, see attached profiler screenshots.

=>

While working on performance improvement in my application, I have found a bottleneck in FirebirdSql.Data.Client.Common.XsqldaMarshaler.GetString(Charset, Byte[]).

After changing:

return value.Replace('\0', ' ').Trim();

To:

return value.TrimEnd('\0', ' ');

I get 4x performance boost.
Please, see attached profiler screenshots.

@firebird-automations
Copy link
Author

Commented by: @cincuranet

Shouldn't it be "value.Trim('\0', ' ');"? Because the first statements trims also the beginning of the string.

@firebird-automations
Copy link
Author

Commented by: Valeriy K. (darkside)

Not sure that we should remove spaces at the beginning, and, I guess, there should not be any "\0" chars at beginning because this is a line-terminator that is placed at the end of string.
Just checked with IB Expert on my test db:

1) run "update tbl_inventory set name = ' new_name ' where id = ?" (note space at beginning and ending of string)
2) run "select name from tbl_inventory where id = ?" and got result " new_name" (note that first space was not trimmed)

But... why we trimming spaces at all? When data is inserted or updated, firebird automatically trim all trailing spaces at the end of string. So the queryes:

1) update tbl_inventory set name = '' where id = ?
2) update tbl_inventory set name = ' ' where id = ?
3) update tbl_inventory set name = ' ' where id = ?

are all identical and if we add unique constraint for field 'name' then all of this three queries will give us "violation of UNIQUE KEY constraint" error.

As conclusion, it looks like the firebird trims trailing spaces when data inserted in DB, and trimming of spaces when fetching data is redundant. So, we just only need to do this:

return value.TrimEnd('\0');

UPDATE:
This example is correct for VARCHAR datatype, but if we are dealing with CHAR, I guess we still need to trim trailing spaces, so I bet that

return value.TrimEnd('\0', ' ');

is more correct in this case.

@firebird-automations
Copy link
Author

Commented by: @cincuranet

But original code was trimming also the beginning of the string. I'm not sure about the reason, it's a protocol (your example with actual SQL might be not relevant), maybe some padding, ... Who knows.

@firebird-automations
Copy link
Author

Commented by: Valeriy K. (darkside)

Let's look at this from the other side. Firebird .NET Client is an adapter between native Firebird library and .NET layer. And as it an adapter - it should only take the data from native methods and pass this data to managed layer. It should not make any modifications to data and changes to protocol because of "adapters" nature.

In current implementation we are trimming possible important spaces from the beginning of the string. Who knows, maybe "some application" logic require those spaces at beginning and we are just removing them.

It's hard to understand for me, why SQL might be not relevant and about what padding you talking about? I took the IB Expert with official Firebird 2.5.1.26351 libraries and ran a few simple SQL queries that displayed me accurate results.

But, if you just don't want to change behavior then "return value.Trim('\0', ' ');" is the right choise. Anyway it works faster than current implementation.

@firebird-automations
Copy link
Author

Commented by: @cincuranet

Yes, for Firebird Embedded it's an adapter. For wire protocol there's native managed implementation.

With IBExpert you don't know what's done between getting data from fbclient.dll and showing it. There might be some trimming as well, because the library might be providing the data in some way. I'm just trying to figure out, whether the change is safe, as it's deep in provider's internals. Looking at where it's being called it looks like a safe change, but who knows. :)

I'll think about it a little more and either change it to Trim or TrimEnd.

@firebird-automations
Copy link
Author

Modified by: @cincuranet

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

Fix Version: vNext [ 10470 ]

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

No branches or pull requests

2 participants