You didn't need those field definitions, right?



  • A customer had been complaining about slow performance loading certain documents in one of our Delphi applications.  Taking a profiler to the offending code, I found that nearly 40% of the time loading these documents was spent in a single system function: @WStrClear.  For those that don't code in Delphi, the only Unicode string type Delphi supported before Delphi 2009 was WideString, and each WideString is allocated by Windows instead of residing in memory the program has already been allocated.  Somehow, a section of code that only explicitly used ANSI strings was creating a large number of WideStrings and immediately discarding them.

     

    In the end, the real slowdown was caused by a combination of legacy code and changes between older versions of Delphi and Delphi 2007.  Somewhere along the line, the type of the FieldName property of dataset fields in the standard database access code was changed from String to WideString.  This slowed down any code accessing fields by name, but more importantly it caused the internal list of fields by their names to change from a StringList to a WideStringList.

    The second link in the chain was a rogue OnScroll event on the in-memory dataset being populated.  Every time a record was added or changes to these in-memory records posted, the OnScroll event would manually look up data from another table.  More accurately, it would look up data from one of several different databases depending on the user's configuration and copy it into a single custom in-memory dataset record.

    The core assumption of the readOtherData code is that if the record we want isn't found in the source table, the in-memory copy of the data must be blank.  To that end, we call a function Clear before even looking for the data (in case the source table doesn't actually have all the fields we want).  Clear closes the in-memory table, removes all the records, and then opens the in-memory table again.  There's only one problem with this: closing the table frees all of the internal field definitions.  Needless to say, closing and then immediately opening is a collosal waste of time.

    The end result of all this was that the application would create 160+ internal field definitions, ask for memory to hold Unicode strings, convert the field names into Unicode strings, read some extraneous data, and immediately ask the operating system to get rid of all 160+ Unicode strings it just created.  Moreover, this would occur at least twice for every line of the document we actually wanted to read - once when we added the line to the in-memory dataset and again when we said we actually wanted to keep it.  This was so time-consuming that a mere 21 records took 7 seconds to load.  Removing the OnScroll event immediately dropped that time to 0.1 seconds.



  • @rampaging-poet said:

    For those that don't code in Delphi, the only Unicode string type Delphi supported before Delphi 2009 was WideString, and each WideString is allocated by Windows instead of residing in memory the program has already been allocated.

    Wait, you're saying every Unicode string is allocated via a syscall??


  • Discourse touched me in a no-no place

    @rampaging-poet said:

    rogue OnScroll event
    Is that something that happens when you read a scroll while playing one of the more stealthy types?



  • @morbiuswilters said:

    @rampaging-poet said:
    For those that don't code in Delphi, the only Unicode string type Delphi supported before Delphi 2009 was WideString, and each WideString is allocated by Windows instead of residing in memory the program has already been allocated.

    Wait, you're saying every Unicode string is allocated via a syscall??

    I bet the WideString type is mainly for COM interoperability / OLE Automation, which requires strings allocated by SysAllocString. I dare to hope the function doesn't actually makes a kernel call until its allocator runs out of memory pages*, but that's still a DLL call that returns memory far away from Delphi's internal heap (resulting in poor memory locality, use of synchronization objects around COM's heap, etc.)

    *I just checked with the debugger, and it calls a function named APP_DATA::AllocCachedMem(), which is a good omen.



  • Yes.  Like Medinoc said, WideString was intended for use with COM.  Newer versions of Delphi (Delphi 2009 and the XE series) have a better type UnicodeString that isn't, but we haven't been able to upgrade because they also made UnicodeString the default string type.  With the continuous push to add new features, taking the time to track down places that would cause issues hasn't happened yet.

    Being stuck at Delphi 2007 also means we don't have generics, but at least we've finally got a foreach loop structure and the IDE crashes much less often than Delphi 5.



  • @rampaging-poet said:

    The second link in the chain was a rogue OnScroll event on the in-memory dataset being populated.  Every time a record was added or changes to these in-memory records posted, the OnScroll event would manually look up data from another table.  More accurately, it would look up data from one of several different databases depending on the user's configuration and copy it into a single custom in-memory dataset record.

    The core assumption of the readOtherData code is that if the record we want isn't found in the source table, the in-memory copy of the data must be blank.  To that end, we call a function Clear before even looking for the data (in case the source table doesn't actually have all the fields we want).  Clear closes the in-memory table, removes all the records, and then opens the in-memory table again.  There's only one problem with this: closing the table frees all of the internal field definitions.  Needless to say, closing and then immediately opening is a collosal waste of time.

     

    This code wouldn't happen to be written by a Russian outsourcing team, would it?  That's been a persistent problem here: they invariably create massive WTFs like this due to a persistent inability to understand the fundamental difference between database and local memory.

     



  • I don't believe so, but that's the way the system was when I started working here.  I'm pretty sure it's because of the way Delphi handles data-aware controls.  By far the easiest way to map UI elements to data is to use the built-in TDB[SomeControl] with a TDatasource and attach a TDataset-descendant class.  This works really well when you know exactly which fields exist and what their names are, but fails when potentially using one form to edit similar data from multiple sources.  As a work-around, we populate an in-memory "database table" with whatever fields we need, pass it to a TDataSource and attach all the controls on a form to that datasource.  It's a bit of a WTF in it's own right, but it works well enough for our purposes.


Log in to reply