[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