[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [T-Merge] wrap long lines issues a comments

From: Oto BREZINA <otik_at_printflow.eu>
Date: Wed, 27 Apr 2011 21:06:32 +0200

On 2011-04-27 19:08, Stefan Küng wrote:
> On 27.04.2011 09:23, Oto BREZINA wrote:
>>
>> On 2011-04-23 11:44, Stefan Küng wrote:
>>> On 23.04.2011 10:05, Oto BREZINA wrote:
>>> Right. I remember that I was thinking of using just one vector instead
>>> of three. But I don't remember why I didn't do it.
>>> I've done some testing and it seems the three vectors really are
>>> identical in all situations I tested.
>>> So I'd say if you want, you can try and use a global vector instead of
>>> three.
>> Here is patch however it does not work. When you start T-Merge it works
>> nice, as soon as you change wrap lines there is no data displayed ?
> Some comments:
> * why a new class for a lock? We already have a very good locking class
> which we use: CReaderWriterLock and the convenience classes
> CAutoReadLock, CAutoWriteLock, CAutoReadWeakLock and CAutoWriteWeakLock.
> * why even a lock? TMerge doesn't use multiple threads so locking
> shouldn't be necessary at all. If you want to block something, don't
> name it lock but e.g., 'block'.
> * why the lock class and then a separate int variable in CBaseView with
> which you do the locking?
just a bad design, comming from code evolution (?) see next paragraphs
where "history" is described.
> * you've added methods like AddBuildLock(), but that one is never used.
> So how does your locking work?
cool. Thing is that I still don't found how to store 'patch' localy and
don't forget about it ...
AddBuildLock need to be just uncomented to activate it.

This "lock" was developed while I tried to cache more screened data.
BuildScreen2View was called that often so speed was reduced dramatically.
But later I start to debug why making screen2view vect static breaks
T-Merge from work correctly and I become not sure if Lock is not a issue
so I commented AddBuildLock out to be sure.

Once I thought Lock works i send it as patch to you - however I forget
to check if AddBuildLock is enabled.
Then I did not removed when sending "patch"-actualy preview patch with
Screen2view set static. (I sent you two patches did I?)

> * don't call it 'build lock' - it isn't for building the project so the
> name doesn't seem correct. I think you want the lock for building the
> screen vector? Name it 'screenvecblock' or something like that.
One of hardest thing on programing is finding a correct name for methods
(not only for me). You have method called (like) buildScreen2viewVect. I
found that this is called too often within one operation.
While debugging clearing vector takes lot of time so I tried to find way
how to speed it up.
Once SW started it is called about 10 times, then views are loaded and
withing loading a later is called about 3-6 times. Don't remember exactly.
So I wrote CBaseView::AddBuildLock and CBaseView::RemoveBuildLock to
reduce number of rebuilding vectors.

Then to reduce possible errors from that (locking and forgot to unlock
because of some code path or whatever) I have wrote wrapper class around
this, so you "can't" forget to unlock building (vectors and other stuff)
called TBuildLock.

Yes it not locking building .exe file but it locks building screen data.
Compiler/linker is not only tool to build things; I have no problem if
you find better name.
However it does not prevent only to build screen2view vect but also some
other things, scrolbars namely (as those depend on screen2view)
> I'm not sure what you want to achieve with this 'lock', but I think it
> isn't really for locking. I think you want to prevent rebuilding the
> screen vector for every view if it was already build by another view. In
> that case, you're actually using some counting mechanism, but not a
> lock. Problem is: you can't just 'count to three' for the views because
> not every view needs to rebuild the vector and not all views are visible
> all the time.
>
> Rebuilding the vector is also done as a response for some window
> messages (like WM_SIZE). In that situation, you can't assume that all
> views will rebuild the vector and can't use counting. And you can't
> block the rebuilding for the same view after it was just built, because
> the WM_SIZE message could arrive twice for one view and then for another
> view, or just twice for one single view and never for another. That
> means you won't gain any performance in that situation - at least I
> can't think of a reliable way...
I used this "Lock" only in OnCreateClient and LoadViews, where
BuildScreen2view method was called more times than needed. You have to
manualy select which methods are rebuilding vect more times in row and
apply locking there.

If you see how to improve code just do it or let me know.
If you don't think this helps in any way (or is too far to be usable),
just let me know I will remove it from my code.
> Stefan

-- 
Oto BREZINA, Printflow s.r.o., EU
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=2725268
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].
Received on 2011-04-27 21:06:49 CEST

This is an archived mail posted to the TortoiseSVN Dev mailing list.