On Tue, Oct 02, 2007 at 08:11:47AM +0200, Abdelrazak Younes wrote: > Andre Poenitz wrote: > >The attached patch replaces the signal/slot connections between Buffer > >and BufferView to LyXView by ordinary delegates. > > > >There has always been only (at most) a single connection of each type > >with known endpoints, so full-blown signal/slot was overkill anyway - > > I am sorry but that's not true for Buffer. With multi-workarea and > multi-view there can be more than one connection to the > Buffer::changed() signal.
The signal was only attached to the LyXView, not to the workarea. I did not change the structure of the code. > >at least given the price of boost's signal/slot: > > > >As boost/signal pulls in roughly 50 kLOC into both Buffer.h and > >BufferView.h which in turn heavily used throughout the code. > >So the change reduces "TLOCC" from 24.6M to 22.5M, i.e. by more than 9%! > > > >There might be a few simplifications possible after that, but I wanted > >to keep the patch small. > > Please wait with that until we understand the full repercussion of that > change. I really sympathize with your compil time reduction goal but I > don't think this is the right solution: > > * For one, you are working around a compiler that do not implement pch > efficiently. I am pretty sure MSVC will not be affected much by your patch. But I am not compiling with MSVC, and if MSVC is "not (adversely) affected" that's completely sufficient. [Apart from that: also MSVC will benefit a bit by not having certain stuff to compile into pch, and finally by not having to compile boost/signal at all] > * Second, did you tried a more recent version of gcc? Gcc 4.2 seems good > on the paper. No, I haven't. But I don't think I will. We have been promised "good gcc" for 15 years or so. I don't believe that anymore. > * Third, I don't know if you remember but I worked an awful lot to get > rid of the WorkArea and LyXView dependency in the core. You are > basically putting them back :-( Heck, no. As I said: I did not change the structure of the code. All dependencies that are in the code now have been there before. And also note that I did use separate interfaces for the delegates (_even_ if only LyXView implements them) exactly for that reason. > On the third point, I understand this is not exactly the same thing. The > problem with boost signals is that they need full visibility of the > objects so we cannot put them in a private implementation. OK, so let's > focus on the signal solution itself: > > - BufferView: I think that most if not all the signal could go with some > more cleanup. This is the right way forward IMO. > > - Buffer: only the changed() signal is important. One solution would be > to have a centralized Buffer manager that will track the WorkAreas > connected to it and update them if need be. This will avoid the need for > this signal in particular. All other signals could and should be removed. > > I hope you are not too upset with my reaction. I am not. I read that that I am 80% going in the right direction, and the remaining bit (multiple buffers) would need a new implementation anyway as we are currently connecting via (a single!) LyXView to the current WorkArea only. The idea of having a BufferManager (which I agree with) is not affected with that patch. Andre'
