On 27.04.2011 21:06, Oto BREZINA wrote:
>> * 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?)
Yes, and I checked both patches.
>> * 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.
I understand how the coding process works. But you really have to clean
up your working copy before you create a patch from it for me to apply.
> 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.
But your patch doesn't contain any changes in those methods. Which
probably means you forgot to include that in your patch.
> 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.
I can't apply this patch (neither the first nor the second one) because
it doesn't make sense. From your comment above I guess you haven't
included everything that's necessary.
When you create a patch, always create it for the whole working copy.
That also forces you to clean up your changes first to avoid getting
stuff into the patch that isn't used or was just added for debugging.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=2725279
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].
Received on 2011-04-27 21:39:23 CEST