On Fri, Jun 10, 2011 at 6:51 AM, tora - Takamichi Akiyama < t...@openoffice.org> wrote:
> Sorry, this mail is too long... No problem, I'll briefly go through your five items one by one: 1. Delegation of the responsibility to choose a type of memory allocator > To achieve both stability and performance at the same time, I would like to > propose "Don't do all of it in the SAL", rather "Delegate certain > responsibility to its users, i.e. programmers." > > Who knows the type of life time of data? SAL does? No. The programmers do. > Do they? Theoretically, they do have the information available that is necessary to choose the most efficient approach that is still correct (depending on how early data can be destroyed again, whether it needs to be multi-thread safe or async safe, etc.). However, in practice, the necessary information is often so big that programmers will make mistakes, choosing approaches that are incorrect. If these are not caught at compile time, and lead to program failure, I think this is a problem (and history shows it to be a rather severe one). > 2. Potential dead lock > A code for crash reporter has a potential, dead lock problem. > http://hg.services.openoffice.org/DEV300/file/tip/sal/osl/unx/backtrace.c > Asynchronous-unsafe functions such as fprintf() are used in the context of > signal handler. > > Consider this situation: > 1. A "Segment violation, aka SEGV" occurs in malloc() or free() due to > memory corruption. Such a function holds the global mutex lock. > 2. The first call of fprintf(), it internally calls malloc() to obtain a > memory area as a text buffer. Then a dead lock occurs. > > For that topic, I would be posing a question later. Yes, OOo's signal handler code is horribly broken. I do not know whether its original authors were unaware of the gross violations of correct programming that are taking place here, but general consensus appears to be that the code happens to work (by accident, I would say) most of the time, and sometimes just locks up even worse than the crash that caused the signal handler to be called in the first place. Anyway, this should be cleaned up one day (and is indeed a topic for a thread of its own). > Please have a look at an additional code fragment in the destructor above: > > if ( Applicatoin::IsMemoryCheckRequested() ) > for ( iterator m_vector ) // Turn them to be a trap > alter_page_attribute( *it, > NO_READ_ACCESS|NO_WRITE_ACCESS|NO_EXEC ); > > 1. soffice.bin is invoked with a new command line option such as > "-memorycheck" > 2. Applicatoin::IsMemoryCheckRequested() returns TRUE. > 3. The memory pages being freed turns to be a trap. > 4. A problematic code mistakenly attempts to read or write data in the > already-freed-memory-area. > 5. The trap sets off the alarm and an interruption is sent by the OS. > 6. A signal handler in the SAL catches the interruption. > 7. A crash report that reveals the exact location of the code is made. > > We have been cultivating thousands of test scenarios for more than a > decade. > Just leave the qatesttool running for a day and night with the option > -memorycheck. > > > 4. Utilizing the cutting-edge technology invented in the 21th century. > > solaris$ cat attempt-of-accessing-the-already-freed-memory-area.c > > #include <stdlib.h> > int main() > { > char *p = (char *) malloc(10); > free(p); > *p = 1; > return 0; > } > > $ cc -g attempt-of-accessing-the-already-freed-memory-area.c > > $ LD_PRELOAD=watchmalloc.so.1 MALLOC_DEBUG=WATCH,RW ./a.out > Trace/Breakpoint Trap (core dumped) > > $ dbx ./a.out core > ... > program terminated by signal TRAP (write access watchpoint trap) > Current function is main > 7 *p = 1; > > Is it easy enough? > Both approaches above (Applicatoin::IsMemoryCheckRequested and watchmalloc) are good for debugging buggy software, but I do not think they are very good answers to the question: "When designing classes like OUString etc., how should efficiency be balanced against safety and maintainability?" I understand that you argue that efficiency should be a priority, and safety can be guaranteed (more or less thoroughly) by testing the code with the mechanisms outlined above. I rather argue that the abstractions available to the programmers should be as safe as possible (even if that costs some efficiency), as programmers will invariably make mistakes, so the potential for mistakes should be minimized. Testing code is all well and important (very much so!), but the tests cannot find all problems (let alone the fact that test coverage for OOo is still rather small). This is probably just another facet of the everlasting dispute between the dynamic and static typing camps. I confess I am sold on the benefits of type theory. 5. 99.9% use cases could be the default. > [...] > In the case above, i.e. in the typical, 99.9% code of OpenOffice.org, I > don't think multithread awareness is required. > > Therefore, the current implementation taking care of multithread-safe, > multicore-processor-awareness might be the waste of energy. > > Again, it would be better, i think, to provide a multithread-unsafe, no > mutex lock involved, simple String class to programmers for 99.9% typical > use cases. > > And also provide highly professional ones for the certain, critical, > race-conditioning use cases. > Again, I see your point. And there is precedent for going the way you describe (Java's synchronized StringBuffer vs. unsynchronized StringBuilder, etc., although the situation is somewhat different there, as multithreaded access to a mutable StringBuffer instance typically requires additional synchronization protocols on top of synchronizing individual methods, anyway, whereas multithreaded access to an immutable OUString does not---in the latter case, synchronization is only needed internally in the OUString instance for correct internal memory management). But would that bring noticeable performance improvements for OOo? > Programmers should be aware of the characteristics of your own data! > > Don't you think so? In general, I think the implications are just too expensive. Cheers, -Stephan -- ----------------------------------------------------------------- To unsubscribe send email to dev-unsubscr...@openoffice.org For additional commands send email to sy...@openoffice.org with Subject: help
