[Ace-users] [ace-users] [usage question] ACE_Log_Msg / ACE_Log_Record - no allocator

Steve Huston shuston at riverace.com
Thu Nov 29 17:27:53 CST 2007


Hi Bogdan,
 
Thanks for looking into this. Your concern about msg_off_ seems valid,
though I didn't track down why it's like that. Your approach to solve
the allocation problems seems to be good.
 
For docs on how to work with the ACE team you can hunt around
http://www.cs.wustl.edu/~schmidt/ACE.html.
 
When you have fixes, you can do one of two things:
 
1. Create a Bugzilla entry for the problem and attach your patches
(unified diff is best) to the entry. This is the safest way to make
sure it doesn't get lost.
 
2. Send the diffs to the ace-users list and ask for someone to
integrate. This can work for very small issues, but this Log_Msg issue
is not going to be that small.
 
Be patient (which you have been all along!) since whomever picks up
your fixes to integrate them will implicitly take responsibility for
any errors that show up on the scoreboard (not many people have access
to the 50 or so platform combinations to test first, so it's not
uncommon for a problem to show up). If people don't have much free
time, they can be a bit hesitant to take responsibility for fixing any
resultant problems. This particular ACE_Log_Msg fix will be more
desireable to fix than usual because of the performance implications,
but still, someone has to take responsibility for it.
 
Thanks,
-Steve

--
Steve Huston, Riverace Corporation
Want to take ACE training on YOUR schedule?
See http://www.riverace.com/training.htm


-----Original Message-----
From: Bogdan Graur [mailto:bgraur at gmail.com] 
Sent: Wednesday, November 28, 2007 3:26 PM
To: Steve Huston
Cc: 'Douglas C. Schmidt'; ace-users at cse.wustl.edu
Subject: Re: [ace-users] [usage question] ACE_Log_Msg / ACE_Log_Record
- no allocator


Hello all,

Thanks Steve for your answer!
I am analyzing now the code in order to do the fix and found that
ACE_Log_Msg::msg_off_ member is declared static. I do not think this
is a good idea considering that ACE_Log_Msg class is used from
different threads (it is used unprotected in ACE_Log_Msg::log()
method) There is also some confusion about it in the code: it is used
some times 'like' a non-static member (this->msg_off_) and some times
'like' a static one (ACE_Log_Msg::msg_off_).

1. Considering that ACE_Log_Msg is kept in TSS I think it is OK to
make member 'msg_off_'  non-static.
2. Proposed fix for the every-time alloc/free (general idea):

a) use an ACE_Log_Record* instead of ACE_TCHAR* to store the log line;

b) declare a new constructor for the ACE_Log_Record that takes as
input the size to allocate for the inner buffer (currently all
ACE_Log_Record ctors allocate as many bytes as ACE_Log_Msg needs but
we shouldn't hard code this dependency between Log_Msg and Log_Record,
a future change of either of one will lead to the change of the other
one);

c) allocate the Log_Record once (in the ctor) and delete once (in
destructor)

Can someone point me to a page describing the process to follow when
posting a fix?

Best regards,
Bogdan


Steve Huston wrote: 

Hi Bogdan,



  

First I'd like to thank you for your prompt reply!

Second, I'd like to apologize for not using the the PRF (I 

thought it is 

not necessary as it was a usage question). Please find it at 

the end of my e-mail.

    



Thanks!



  

I know now that each thread has its own ACE_Log_Msg instance kept in

    



  

TSS. Thanks for pointing this out!



But, inside the method:

    

ssize_t

ACE_Log_Msg::log (const ACE_TCHAR *format_str,

                  ACE_Log_Priority log_priority,

                  va_list argp)

      

an ACE_Log_Record is instantiated on the stack for which the 

constructor 

used allocates 'another' 4 KB of data. The ACE_Log_Msg 

internal buffer 

is afterwards copied ( ACE_OS::strcpy) in the internal buffer of 

ACE_Log_Record.

    



Right... The ACE_Log_Record here used to have it's data as a member

array, and so was allocated off the stack. This was changed, from what

I can see, at:



Tue Mar 28 21:30:01 UTC 2006  jiang,shanshan

 <mailto:shanshan.jiang at vanderbilt.edu>
<shanshan.jiang at vanderbilt.edu>



        * ace/Log_Msg.{h,cpp}

        * ace/Log_Record.{cpp,h,inl}:

          Updated these files to solve the stack overflow problem in

ACE_Log_Msg

          and ACE_Log_Record. Moves buffers that can be large in stack

in malloced

          memory.

          Thanks to qwerty <qwerty0987654321 at mail dot ru> for

motivating and

          suggesting the fix to this problem.



  

I made a simple test program, ran 'gdb' on it and found that 

each time 

this function is invoked, the ACE_Log_Record's constructor 

gets called 

and its underlying data is allocated at a different address 

(to try to 

rule out some overloaded 'operator new' I couldn't identify).



Please tell me if there is something I missed. Thank you!

    



Nope, you got it right, but the above work to remove the MAXLOGMSGLEN

size restriction could have been done much better to avoid the

every-time alloc/free. I believe this can be done in a much more

efficient manner. One possible idea is to replace the msg_ array in

ACE_Log_Msg with a ACE_Log_Record, but I haven't traced all the

possibilities out to see if this is a good solution. If you can work

out the details of this, that would be great. If you don't have

time/energy to do this, please record this info in Bugzilla or

consider partnering with someone who can analyze the situation and

develop a good fix for it.



Thanks very much for raising this and analyzing the source of the

excessive memory allocations.



Best regards,

-Steve



--

Steve Huston, Riverace Corporation

Want to take ACE training on YOUR schedule?

See http://www.riverace.com/training.htm



  

  

      

On my understanding each time you log something using ACE_Log_Msg

        



  

another ACE_Log_Record is created which in turn creates 

        

its underlying 

    

data (4 KB) on the heap. This can happen very often and may have

        

a 

  

negative impact on the overall performance of the application.

    

        

No, that's incorrect.  the 4 KB of data is allocated using

thread-specific storage (see the discuss in the POSA2 book at

www.cs.wustl.edu/~schmidt/POSA/POSA2/), so the 4 KB is 

      

allocated only

    

once for each thread.



  

      

Is there a way to control the allocation of ACE_Log_Record 

        

data without 

    

overloading the global new operator?

    

        

No, but this shouldn't be necessary, as per the discussion above.



Thanks,



Doug

  

      

_______________________________________________

ace-users mailing list

ace-users at mail.cse.wustl.edu

http://mail.cse.wustl.edu/mailman/listinfo/ace-users



    







  


-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://list.isis.vanderbilt.edu/pipermail/ace-users/attachments/20071129/9ab18ddf/attachment.html 


More information about the Ace-users mailing list