How to Thread Safe my TObject List and optimize it ?

I'm creating multi thread program which mainly processing list of incoming data and output it into another list ready to send.

So now my thread processing mainly data from TObjectList.

But the problem with TObjectList in my program is it is strickly array like list instead of easily grow and shrink linked list. This is problem when a list being accessed by two or more thread when one still browse the list the other delete one of the item. I'm mostly doing the list browse like this :

for i := 0 to ObjectList.Count-1 do
begin
  AObject := ObjectList.Items[i];
end;

So now I create my own Object List which I think better suit my need rather than using strict FOR DO, I can using WHILE NOT EOF DO.
It is run great, but problem occur when I'm delete one of the item. What is the problem in my code below :

unit UObjectListX;

interface
uses
  SysUtils;

type
  TPObject = ^TObject;

  TPRec = ^TRecord;
  TRecord = record
              Prev: TPRec;
              Nxt: TPRec;
              Data: TObject;
            end;

  TObjectListX = class(TObject)
  private
    m_Head: TPRec;
    m_Sync: TMultiReadExclusiveWriteSynchronizer;
    m_Run: TPRec;
    m_Count: cardinal;

  public
    constructor Create;
    destructor Destroy; override;
    procedure Add(AObject: TObject);
    procedure Delete(AObject: TObject);
    procedure Start;
    procedure Next;
    function EOF: boolean;
    function Item: TObject;
    procedure Pack;
    function Count: cardinal;
  end;


implementation

{ TObjectListX }

constructor TObjectListX.Create;
begin
  inherited;
  m_Head := nil;
  m_Sync := TMREWSync.Create;
  m_Count := 0;
end;

destructor TObjectListX.Destroy;
var
  Run: TPRec;

begin
  Run := m_Head;
  while Run <> nil do
  begin
    m_Head := Run.Nxt;
    dispose(Run);
    Run := m_Head;
  end;
  FreeAndNil(m_Sync);
  inherited;
end;

procedure TObjectListX.Add(AObject: TObject);
var
  anew: TPRec;

begin
  m_Sync.BeginWrite;
  try
  New(anew);
  anew.Prev := nil;
  anew.Data := AObject;
  if m_Head = nil then
  begin
    m_Head := anew;
    m_Head.Nxt := nil;
  end
  else
  begin
    anew.Nxt := m_Head;
    m_Head.Prev := anew;
    m_Head := anew;
  end;
  inc(m_Count);
  finally
    m_Sync.EndWrite;
  end;
end;

procedure TObjectListX.Delete(AObject: TObject);
var
  Run: TPRec;

begin
  m_Sync.BeginWrite;
  try
  Run := m_Head;
  while Run <> nil do
  begin
    if Run.Data = AObject then
    begin
      if Run = m_Head then m_Head := Run.Nxt;
      if Run.Prev <> nil then Run.Prev.Nxt := Run.Nxt;
      if Run.Nxt <> nil then Run.Nxt.Prev := Run.Prev;
      Dispose(Run);
      dec(m_Count);
      break;
    end;
    Run := Run.Nxt;
  end;
  finally
    m_Sync.EndWrite;
  end;
end;

procedure TObjectListX.Start;
begin
  m_Sync.BeginWrite;
  try
    m_Run := m_Head;
  finally
    m_Sync.EndWrite;
  end;
end;

procedure TObjectListX.Next;
begin
  m_Sync.BeginWrite;
  try
    if Assigned(m_Run) then m_Run := m_Run.Nxt;
  finally
    m_Sync.EndWrite;
  end;
end;

function TObjectListX.EOF: boolean;
begin
  Result := true;
  m_Sync.BeginRead;
  try
    if m_Run <> nil then Result := false;
  finally
    m_Sync.EndRead;
  end;
end;

function TObjectListX.Item: TObject;
begin
  Result := nil;
  m_Sync.BeginWrite;
  try
    if m_Run <> nil then Result := m_Run.Data;
  finally
    m_Sync.EndWrite;
  end;
end;

procedure TObjectListX.Pack;
var
  Run: TPRec;

begin
  m_Sync.BeginWrite;
  try
  Run := m_Head;
  while Run <> nil do
  begin
    if Run.Data = nil then
    begin
      if Run = m_Head then m_Head := Run.Nxt;
      if Run.Prev <> nil then Run.Prev.Nxt := Run.Nxt;
      if Run.Nxt <> nil then Run.Nxt.Prev := Run.Prev;
      Dispose(Run);
      dec(m_Count);
    end
    else
      Run := Run.Nxt;
  end;
  finally
    m_Sync.EndWrite;
  end;
end;

function TObjectListX.Count: cardinal;
begin
  Result := m_Count;
end;

end.


I like the way I'm using it because it's almost mimic TObjectList, just need a bit change on implementation for example :

ObjectList.Start
while not ObjectList.EOF do
begin
  AObject := ObjectList.Item;
  ObjectList.Next;
end;

BUT, if you Expert think that you can create another alot BETTER TObjectList for me, which can be easily implemented such as usual TObjectList (so I change code very little to implement it), I'm really LIKE that.
ALSO if my code is already the right way, please fix the bug and please OPTIMIZE it if you can into INLINE assembly for ALOT FASTER execution, since I will use this ALOT.

Thank You SO MUCH ^^ :)
comicboyAsked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

BlackTigerXCommented:
when you delete a record, why do you search for it?

if you already have a pointer and this is a both ways linked list, you can just do something like:

if (CurrentRecord <> Head) then
begin
  CurrentRecord.Prev.Next:=CurrentRecord.Next;
  CurrentRecord.Next.Prev:=CurrentRecord.Prev;
  Dispose(CurrentRecord)
else
if (CurrentRecord.Next<>nil) then
begin
  Head:=CurrentRecord.Next;
  Dispose(CurrentRecord)
end
else
  Dispose(CurrentRecord)

didn't test it, but that's the idea
0
arnismitCommented:
comicboy

gotto work in my garden right now,

but im checking the code later on tonight

i'll change your code this time instead of giving you a full example.
right ???

greetings
arni
0
comicboyAuthor Commented:
BlacTigerX sorry that is not the answer I seek ^^

Arni, where is you ? ^^

I just want someone pin point what is WRONG with my code which cause error when one of the element being deleted.
Also if any expert may kindly create a whole NEW TObjectList forme which better than mine, otherwise just optimize it for speediest possible i.e. make some tobe inline assembly.

Or maybe this question doesn't get anymore attention from expert, maybe I will delete this one and RE-ASK again the same question, so another expert will take notice ??

Thank You
0
Cloud Class® Course: Certified Penetration Testing

This CPTE Certified Penetration Testing Engineer course covers everything you need to know about becoming a Certified Penetration Testing Engineer. Career Path: Professional roles include Ethical Hackers, Security Consultants, System Administrators, and Chief Security Officers.

arnismitCommented:
comicboy,

sorry for the delay so far

i have been very busy lately

i was about to check this question right now.

i will make it thread safe for you

hold on
0
arnismitCommented:
comicboy,

i looked over your code,

but i will still stand my point in your previous question that a queue would be easier to use....

i will try to transform your code into that example...

you will have to change a bit your code.

what i would like to suggest is that you send me your code via email to me, i will post a complete update of all changed parts here,

is that an option ?

greetings,
arni
0
arnismitCommented:
i will first try to fix your list code your gave in here ok?
0
arnismitCommented:
Comicbpy

here is a start for it

hope you can use this one..

if something is stil unclear please ask!

greetings
arni

unit NewList;

interface

Uses classes, Contnrs, sysUtils;
(*
note in advance:

you use in your list some functions that are not needed with this code
the add function in your list is here PushItem
the delete function and the item function are combined into the PopItem function
the next and eof functions are not needed anymore
you could implement a count function if needed...
to see if a list is empty or not use the peek method
*)
type
// pre-definitions
  TMyQueue = Class; // this is corresponds to your Record structure
                    // the queue takes care of your linking between the previous and the next item in the list

  TMyQueueItem = Class;  // this is corresponding to your data object in your record structure

// full declarations
  TMyQueue = Class(TObject)
    FSyncObj: TMultiReadExclusiveWriteSynchronizer; // this objects makes your record structure threadsafe
    FObjQueue: TObjectQueue; // this is the object where your actual itmes are stored
  public
    constructor Create;
    destructor Destroy; Override;

    function PopItem: TMyQueueItem;
    procedure PushItem(newMyQueueItem: TMyQueueItem);
    function PeekItem: TMyQueueItem;      // used to see if there is an item waiting
  End;

(*
  Use the function PopItem to get a next item that you should prcess
  It returns NIL when there is no object to process
  It returns a TMyQueueItem when it has a item to process
  tis function also removes the item from the list, so it wont be process twice

  example how to use it:

  var
    MyItemToProcess: TMyQueueItem;
  begin
    MyItemToProcess := TheQueue.PopUtem; // get a new item to process
    If assigned(MyItemToProcess) then // check whete the item is filled with an object
    begin
      // here do the processing for the data object.....
    end;
  end;


  Use the function PushItem to put items in the list.
  Somewhere in your application you are putting items together in the "to be processed list"
  You should use this function there

  example how to use it:

  example 1

  var
    MyNewItemForTheList: TMyQueueItem;
  begin
    MyNewItemForTheList := TMyQueueItem.Create;   // create an empty dataholder
    Try
      MyNewItemForTheList.data_item1 := 'dfkljskldfjs';  // add the data you want to
      MyNewItemForTheList.data_item2 := 2;

      TheQueue.PushItem(MyNewItemForTheList);  // push the item to the queue list for processing
      MyNewItemForTheList := Nil;             // remove the local reference to the object sinc you dont need to free it in here
    Except
      MyNewItemForTheList.Free;           //  if something went wrong the item isnt added to the list, so free it.
    End;
  end;

  example 2

  this is a very short way to do it

  begin
    TheQueue.PushItem(TMyQueueItem.Create('dfkljskldfjs', 2));  // create the item with data and put it on the list
  end;

*)

// this class holds your data for each item in the list

  TMyQueueItem = Class(TObject)
  private
    // You can store as much data in a Item as you want, just make more
    // properties
    FData_Item1: String;
    FData_Item2: Integer;
  public
    // use this constructor if you want to create an item with data
    Constructor Create(aData_Item1: String; aData_Item2: Integer); Overload;

    // use this constructor if you want to create an item without data
    Constructor Create; Overload;

    Property Data_Item1: String Read FData_Item1 Write FData_Item1;
    Property Data_Item2: Integer Read FData_Item2 Write FData_Item2;
  End;

Implementation

{ TMyQueueItem }

constructor TMyQueueItem.Create(aData_Item1: String; aData_Item2: Integer);
begin
  Inherited Create;
  FData_Item1 := aData_Item1;
  FData_Item2 := aData_Item2
end;

constructor TMyQueueItem.Create;
begin
  Create('', 0);
end;

{ TMyQueue }

constructor TMyQueue.Create;
begin
  inherited;
  FSyncObj := TMREWSync.Create;
  FObjQueue := TObjectQueue.Create;
end;

destructor TMyQueue.Destroy;
begin

  FreeAndNil(FObjQueue);
  FreeAndNil(FSyncObj);
  inherited;
end;

function TMyQueue.PeekItem: TMyQueueItem;
var
  aObj: TObject;
begin
  Result := Nil;
  FSyncObj.BeginRead;
  Try
    aObj := FObjQueue.Peek;

  Finally
    FSyncObj.EndRead;
  End;

  If Assigned(aObj) Then Result := TMyQueueItem(aObj);
end;

function TMyQueue.PopItem: TMyQueueItem;
Var
  aObj: TObject;
begin
  Result := Nil;

  FSyncObj.BeginWrite;
  Try
    aObj := FObjQueue.Pop;
  Finally
    FSyncObj.EndWrite;
  End;

  If Assigned(aObj) Then
    Result := TMyQueueItem(aObj);
end;

procedure TMyQueue.PushItem(newMyQueueItem: TMyQueueItem);
begin
  FSyncObj.BeginWrite;
  Try
    FObjQueue.Push(newMyQueueItem);
  Finally
    FSyncObj.EndWrite;
  End;
end;

end.
0
comicboyAuthor Commented:
Thank You Arni,

Anyway by using queue doesn't mean a FIFO or LIFO right ?

Because I want the List also can be :
1. Get item anywhere from the list
2. Get item without delete them
3. Delete item without get them

Thank You
0
comicboyAuthor Commented:
Also :
4. I can search through the list to get the Item I want

TListObject is good example, but the problem with it is : array like list. I don't want that.

So can you give me the example how to use your queue upon my 4 task above ?

Thank You
0
comicboyAuthor Commented:
Hi Arni, please don't be confused this current question with my previous question about Thread (the one with solution Sleep(xxx) ), because the case is different now, this is different question on different case then our previous conversation (well maybe not a conversation ^^).

I need to do like this :

For example I have List of TObject which each object contain a string for ID, for instance the list contain an Object with string ID "TEST" on it's "15th position" then I want to do this :

AObject := Get('TEST'); //get object which contain string 'TEST' (in this example the object contain string var) without deleting them
Delete('TEST'); //delete object (without need to get them) which contain string 'TEST'

So the function doing search for ID match within the list then do either GET or DELETE, but noth both at once because often I still need the Object which I've already GET.

Please give me example how to achieve that with the queue, because that is what I needed to do the ObjectList for.
0
DeerBearCommented:
Hi.

The simplest way that comes to mind is to use the same mechanics used in TThreadList.
Browse the VCL sources and "copy" :D

Cheers,

Andrew
0
comicboyAuthor Commented:
Hi DeerBear,

Thank You for your guidance, what file (*.PAS) which contain TThreadList, because I couldn't find any file called TThreadList.pas there.
0
comicboyAuthor Commented:
Hi DeerBear,

Well I found it within classes unit. But I don't quite understand the code also it almost looks the same as TObjectList in operation (array like list), which I'm trying avoid.

Because what I don't like about this array like list is, to traverse it I should use something like this :

for i := 0 to List.Coun-1 do
begin
  if AnObject(List.Items[i]).Name = 'TEST' then
  begin
     ..... //do something
     break;
  end;
end;

While that is what I'm implement previously (before I'm creating my own list as sampled at top of this question) using TObjectList, the dangerous using this array like list is, when another Thread suddenly delete on of its Item and Pack them, then suddenly current Thread still running the FOR DO count mismatch the items COUNT, in Threading environment this cause undesired situation error.

Thats why I'm trying to create my own list, which does not depen on counting through the list when traversing such as using double link list as my sample above. But my code seems not correct, because it also causing error when an item being deleted, I'm suspect that I didn't lock the code safe enough for multi threading situation.

Also since I will use this code very much, please optimize it to the most. Thank You.
0
DeerBearCommented:
ComicBoy,

I see several problems in your implementation.

This should work much better:

uses Classes, SyncObjs, Contnrs;

Type

  TThreadedObjectList = class
  private
    FList : TObjectList;
    CriticSect : TCriticalSection;
  public
    constructor Create;
    destructor Destroy;override;
    function LockList : TObjectList;
    procedure UnlockList;
    procedure ThreadedDelete(AnObj : TObject);
  end;

constructor TThreadedObjectList.Create;
begin
  inherited Create;
  FList := TObjectList.Create(True); // Unless you need to keep objects detatched, in that case use False.
  CriticSect := TCriticalSection.Create;
end;

destructor TThreadedObjectList.Destroy;
begin
  CriticSect.Free;
  FList.Free;
  inherited Destroy;
end;

function TThreadedObjectList.LockList : TObjectList;
begin
  CriticSect.Enter;
  result := FList;
end;

procedure TThreadedObjectList.UnlockList;
begin
  CriticSect.Leave;
end;

procedure TThreadedObjectList.ThreadedDelete(AnObj : TObject);
var TempList : TObjectList;
begin
  LockList.Remove(AnObj);
  UnlockList;
end;


How about this one?

Cheers,

Andrew
0
arnismitCommented:
comicboy

the queue is not used for operations like that. Because the queue assumes when you pop an item you process it and you dont need it anymore.

a work arround would be to add the item again to the queue with the push method.

i do not agree with using a threadlist anyways. Andrew, what problems did you find in comicboy's code ? beside the things that might not be the best way to do it ?

greetings
arni
0
comicboyAuthor Commented:
DeerBear;

Well it seem, you just use the same TObjectList but in secure thread environment as within TThreadList, I like the Idead on securing it, but as I said I don't like to use array like list in this case apply to TObjectList as I mentioned before.

The problem will occur with array like list in multi thread situation, when the same list accessed by several thread which when one thread still traversing it using FOR..DO, the other thread DELETE one of its item and PACK it. This ofcourse causing error on the traversing Thread because the COUNT no longer valid.

And so I have idea on creating my own list (a double linked list sampled at top), but I don't know why, it also causing error when one of the thread delete its item. Maybe my code contain error, but also maybe it is not secured in multi thread environment.

I tried to use Thread read write sync as Arni teach me, but thats not help, maybe I'm wrong on implementing that.

Maybe I will try to incorporate what Andrew show in the code above, but change the TObjectList with my own code.

Anyway can experts please look at my code and optimize it ? (preferably into inline assembly for faster execution)

Thank You
0
comicboyAuthor Commented:
Arni;

I think I can't use LIFO like queue, because sometime I need to get item in the middle and get that via searching the ID of the item not just pop the Last entry of item. Also I want to freely delete an item anywhere in the list, while I don't want to delete item when I get it, because most of the time item I've get still needed for later get, unless it doesn't needed anymore and that time it will be simply deleted without need to get it.

What I need is this simple:
- A list can hold TObject
- Multi thread secure, so while other traversing, other thread can safely Delete or Change an item on it
- Search an item needed from anywhere within list by traversing it and compare the ID of the item (to find the exact item needed)
- Delete an item anywhere on the list

Thank You ^^
0
comicboyAuthor Commented:
From Delphi Help :

Use TMultiReadExclusiveWriteSynchronizer to guard access to memory in a multi-threaded application. Unlike a critical section, which blocks all other threads from reading or writing its associated memory, TMultiReadExclusiveWriteSynchronizer allows multiple threads to read from the protected memory simultaneously, while ensuring that any thread writing to the memory has exclusive access.

In applications where threads read from an object or variable frequently and only write to it occasionally, using the multi-read exclusive-write synchronizer instead of a critical section can result in considerable performance improvement.

So TMREWSync is better, but well I didn't test critical section yet...

I'll report the result...
0
comicboyAuthor Commented:
This is sample of Error I've got :

Project myprogram.exe raised exception class EAccessViolation with with message 'Access violation at address 0040339A in moduule myprogram.exe'. Read of address 6D696E41'. Process stopped. Use Step or Run to continue.

Also I read from Delphi Help :

Use TCriticalSection to safeguard operations that may be disrupted if another thread executes certain sections of code before the operation is completed. Critical sections work like gates that allow only a single thread to enter at a time. Because they block the execution of other threads, overusing critical sections can seriously interfere with performance.

Critical sections must be global in scope so that they are available to all threads. In every thread, any action that could interfere with the operations safeguarded by the critical section should only take place after calling the Acquire or Enter method. Threads that do not use the critical section for relevant operations can introduce bugs.

Well it looks will slow down and introduce bugs, not promising :p but I will try it anyway...
0
comicboyAuthor Commented:
I'm trying it, well the result the same as using TMREWSync = error EAccessViolation, I don't know why...

Sorry to inform that using either TMREWSync or TCriticalSection causing my program got EAccessViolation from Start (not only when deleting item), but when I change back using TObjectList, only got EAccessViolation when delete item.

Thank You
0
arnismitCommented:
hey comicboy

i build something for you tonight with these definitions:

What I need is this simple:
- A list can hold TObject
- Multi thread secure, so while other traversing, other thread can safely Delete or Change an item on it
- Search an item needed from anywhere within list by traversing it and compare the ID of the item (to find the exact item needed)
- Delete an item anywhere on the list

i'll do it tonight, since im leaving tomorrow for a short holiday.

greetings
arni
0
comicboyAuthor Commented:
Thank You Arni ^^ wait for yours
0
arnismitCommented:
sorry comicboy, i had no time fixing the object on thursday , busy packing and stuff

but im back now. thank you for your patience

greetings,
arni
0
arnismitCommented:
hi comicboy,

is this what you need ?

unit threadsafeobjectlist;

interface

uses
  Classes, SysUtils;

Type
  TTSObjList = Class(TObject)
  private
    FList: TStringList;
    FMREWSync: TMultiReadExclusiveWriteSynchronizer;

  public
    Constructor Create;
    Destructor Destroy; Override;

    procedure AddObject(aID: String; aObj: TObject);
    procedure DeleteObject(aId: String);
    function FindObject(aId: String): TObject;
    procedure Clear;
  End;

implementation


{ TTSObjList }

procedure TTSObjList.AddObject(aID: String; aObj: TObject);
begin
  FMREWSync.BeginWrite;
  Try
    FList.AddObject(aID, aObj);
  finally
    FMREWSync.EndWrite;
  end;
end;

procedure TTSObjList.Clear;
var
  IndxNo: Integer;
begin
  FMREWSync.BeginRead;
  try
    for IndxNo := Pred(FList.Count) DownTo 0 Do
      DeleteObject(FList[IndxNo]);
  finally
    FMREWSync.EndRead;
  end;
end;

constructor TTSObjList.Create;
begin
  Inherited Create;
  FList := TStringList.Create;
  FList.Sorted := True;
  FList.Duplicates := dupIgnore;

  FMREWSync := TMREWSync.Create;
end;

procedure TTSObjList.DeleteObject(aId: String);
var
  IndxNo: Integer;
begin
  FMREWSync.BeginRead;
  Try
    If FList.Find(aId, IndxNo) Then
    Begin
      FMREWSync.BeginWrite;
      try
        FList.Objects[IndxNo].Free;
        FList.Objects[IndxNo] := Nil;
        FList.Delete(IndxNo);
      finally
        FMREWSync.EndWrite;
      end;
    end;
  finally
    FMREWSync.EndRead;
  end;
end;

destructor TTSObjList.Destroy;
begin
  FMREWSync.Free;
  Clear;
  FList.Free;
  inherited;
end;

function TTSObjList.FindObject(aId: String): TObject;
var
  IndxNo: Integer;
begin
  FMREWSync.BeginRead;
  Try
    Result := Nil;
    If FList.Find(aId, IndxNo) Then
      Result := FList.Objects[IndxNo];
  finally
    FMREWSync.EndRead;
  end;
end;

end.

good luck
arni
0
comicboyAuthor Commented:
Hi Arni, thank you for the code above, I will try that though that still using Array like List, but well I will try it first, hope that wil do the work :)

Please wait for a while since I have another assignment this week ^^
I'll get my hand on it as soon as possible ^^

Thank You
0
arnismitCommented:
ComicBoy,

take it easy. I will notice when you are back on it.
one point. Depending on what you exactly want to build
this way is possibly not the most effective way....

this code can be optimized, but that completly depends on
how much (and what kind of) data you are storing in the list, if you are willing to switch to another memory manager (delphi's mm is slow on large strings) a performance boost up to 2000 % is possible
a TStringlist is not most optimized way, but by far the most easiest way (as you maybe noticed).

Let me know if you still need something.
greetings,
arni
0
cubudCommented:
You could create a critical section in the list and use Enter/Leave before using it.  You should also look at TMultiReadExclusiveWriteSynchronizer if you would like to enable multiple reading threads but only a single updater.


Pete
===
Delphi articles
http://www.howtodothings.com/ViewSubCategory.aspx?SubCategory=41
0
comicboyAuthor Commented:
Hi arni, I read that you and some experts here say that my way of list is not the best way to do it, which part is it ? could you please optimize that part for faster execution please ?

Anyway Arni, I'm using your queue way replacing my previous question TStringList method.

Thank You
0
arnismitCommented:
Comicboy,

if you are not using a tstringlist anymore you have optimized at least a bit already.
what you should consider is, if  the queue is really what you want. If you want to search in the list, look up items, add items and delete items you can change a simple list object like a tstringlist or a tobjectlist with a linkedlist. Not to a queue, because a queue is slow for that purpose. Moving over to a linked list will increase performance, but you wont see increasing performance unless you are storing over hundred thousend of items.

As i explained before, working with string concat's is slow in delphi unless you use another memory manager like Nexus DB memory manager. I have experience with that one, and it boosts my ISAPI web apps with about 2000 %. But thats only because borlands isapi implementation is a bit slow.

But best way to optimize is first to determine what performance your app should have. if your app doenst apply for this, then look for ways to boost it. you can spend a lot of time in winning a few miliseconds, but is it really worth the efforts?

Is there, at this point still some code that is not working for you ? Do you still have any questions concercning the topic of this question ?

Greetings,
Arni

0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
comicboyAuthor Commented:
Your Queue is better rathern than using TStringList and get Strings[0] and delete it, better use pop.

Anyway what I'm asking about optimize is this code :

unit UObjectListX;

interface
uses
  SysUtils;

type
  TPObject = ^TObject;

  TPRec = ^TRecord;
  TRecord = record
              Prev: TPRec;
              Nxt: TPRec;
              Data: TObject;
            end;

  TObjectListX = class(TObject)
  private
    m_Head: TPRec;
    m_Sync: TMultiReadExclusiveWriteSynchronizer;
    m_Run: TPRec;
    m_Count: cardinal;

  public
    constructor Create;
    destructor Destroy; override;
    procedure Add(AObject: TObject);
    procedure Delete(AObject: TObject);
    procedure Start;
    procedure Next;
    function EOF: boolean;
    function Item: TObject;
    procedure Pack;
    function Count: cardinal;
  end;


implementation

{ TObjectListX }

constructor TObjectListX.Create;
begin
  inherited;
  m_Head := nil;
  m_Sync := TMREWSync.Create;
  m_Count := 0;
end;

destructor TObjectListX.Destroy;
var
  Run: TPRec;

begin
  Run := m_Head;
  while Run <> nil do
  begin
    m_Head := Run.Nxt;
    dispose(Run);
    Run := m_Head;
  end;
  FreeAndNil(m_Sync);
  inherited;
end;

procedure TObjectListX.Add(AObject: TObject);
var
  anew: TPRec;

begin
  m_Sync.BeginWrite;
  try
  New(anew);
  anew.Prev := nil;
  anew.Data := AObject;
  if m_Head = nil then
  begin
    m_Head := anew;
    m_Head.Nxt := nil;
  end
  else
  begin
    anew.Nxt := m_Head;
    m_Head.Prev := anew;
    m_Head := anew;
  end;
  inc(m_Count);
  finally
    m_Sync.EndWrite;
  end;
end;

procedure TObjectListX.Delete(AObject: TObject);
var
  Run: TPRec;

begin
  m_Sync.BeginWrite;
  try
  Run := m_Head;
  while Run <> nil do
  begin
    if Run.Data = AObject then
    begin
      if Run = m_Head then m_Head := Run.Nxt;
      if Run.Prev <> nil then Run.Prev.Nxt := Run.Nxt;
      if Run.Nxt <> nil then Run.Nxt.Prev := Run.Prev;
      Dispose(Run);
      dec(m_Count);
      break;
    end;
    Run := Run.Nxt;
  end;
  finally
    m_Sync.EndWrite;
  end;
end;

procedure TObjectListX.Start;
begin
  m_Sync.BeginWrite;
  try
    m_Run := m_Head;
  finally
    m_Sync.EndWrite;
  end;
end;

procedure TObjectListX.Next;
begin
  m_Sync.BeginWrite;
  try
    if Assigned(m_Run) then m_Run := m_Run.Nxt;
  finally
    m_Sync.EndWrite;
  end;
end;

function TObjectListX.EOF: boolean;
begin
  Result := true;
  m_Sync.BeginRead;
  try
    if m_Run <> nil then Result := false;
  finally
    m_Sync.EndRead;
  end;
end;

function TObjectListX.Item: TObject;
begin
  Result := nil;
  m_Sync.BeginWrite;
  try
    if m_Run <> nil then Result := m_Run.Data;
  finally
    m_Sync.EndWrite;
  end;
end;

procedure TObjectListX.Pack;
var
  Run: TPRec;

begin
  m_Sync.BeginWrite;
  try
  Run := m_Head;
  while Run <> nil do
  begin
    if Run.Data = nil then
    begin
      if Run = m_Head then m_Head := Run.Nxt;
      if Run.Prev <> nil then Run.Prev.Nxt := Run.Nxt;
      if Run.Nxt <> nil then Run.Nxt.Prev := Run.Prev;
      Dispose(Run);
      dec(m_Count);
    end
    else
      Run := Run.Nxt;
  end;
  finally
    m_Sync.EndWrite;
  end;
end;

function TObjectListX.Count: cardinal;
begin
  Result := m_Count;
end;

end.


Because I see some experts here told that code is not the best way to do it, could you please pin point what part can be optimized for better performance.

Thank You
0
arnismitCommented:
i will look into it :)

i'll let you know
0
comicboyAuthor Commented:
ok thank You ^^
0
arnismitCommented:
comicboy,

i havebeen away for a while.

is there still anything you want in here ?
anything i can do for you related to this topic ?

greetings
arni
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Delphi

From novice to tech pro — start learning today.

Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.