Issue Details (XML | Word | Printable)

Key: DNET-452
Type: Improvement Improvement
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Jiri Cincura
Reporter: Valeriy K.
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
.NET Data provider

[PATCH] Performance improvements in fetch of string data

Created: 10/Aug/12 11:18 AM   Updated: 05/Sep/12 08:11 AM
Component/s: ADO.NET Provider
Affects Version/s: 2.7.7
Fix Version/s: 3.0.0.0

File Attachments: None
Image Attachments:

1. after.png
(70 kB)

2. before.png
(73 kB)
Environment: Windows 7 x64, .NET 4.0


 Description  « Hide
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.

 All   Comments   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Jiri Cincura added a comment - 15/Aug/12 12:43 PM
Shouldn't it be "value.Trim('\0', ' ');"? Because the first statements trims also the beginning of the string.

Valeriy K. added a comment - 15/Aug/12 06:05 PM - edited
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.

Jiri Cincura added a comment - 15/Aug/12 06:55 PM
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.

Valeriy K. added a comment - 15/Aug/12 07:39 PM - edited
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.

Jiri Cincura added a comment - 16/Aug/12 11:53 AM
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.