Login | Register
My pages Projects Community openCollabNet

Discussions > dev > Re: [T-Merge Patch] Lock build to reduce build time

Project highlights: :. Download .: :. Support .: :. FAQ .: :. Translations .: :. Donate .: :. Report Bug .:

tortoisesvn
Discussion topic

Hide all messages in topic

All messages in topic

Re: [T-Merge Patch] Lock build to reduce build time

Author otik
Full name Oto BREZINA
Date 2011-04-28 14:07:44 PDT
Message On 2011-04-28 21:39, Oto BREZINA wrote:
> On 2011-04-28 20:36, Stefan Küng wrote:
>> Should work as of r21234.
> Works great for me! You just speed up part of T-merge by at least 5 ...
Ok, I just try change wrap lines and/or colapse scrolbars are not
updated (nor locator bar)
Attached patch address this.
* update bars, locator after build screen2view vect
* applied modified patch from Dmitry - reduce code duplication
(http://tortoisesvn.t​igris.org/ds/viewMes​sage.do?dsForumId=75​7&dsMessageId=27​25422)
* don't use FindScreenLineForViewLine in OnLButtonDblClk to get value we
already know
* sugestion move test for m_pViewData into DoRebuild
>> Stefan

--
Oto BREZINA, Printflow s.r.o., EU
Attachments

Re: [T-Merge Patch] Lock build to reduce build time

Author otik
Full name Oto BREZINA
Date 2011-04-28 12:40:03 PDT
Message On 2011-04-28 20:36, Stefan Küng wrote:
> Should work as of r21234.
Works great for me! You just speed up part of T-merge by at least 5 ...
> Stefan
>

--
Oto BREZINA, Printflow s.r.o., EU

Re: [T-Merge Patch] Lock build to reduce build time

Author steveking
Full name Stefan Küng
Date 2011-04-28 11:36:41 PDT
Message On 28.04.2011 12:28, Oto BREZINA wrote:
> On 2011-04-28 12:23, Stefan Küng wrote:
>> On Thu, Apr 28, 2011 at 09:30, Oto BREZINA<otik@prin​tflow.eu> wrote:
>>>
>>> Sounds good to me, I did not want to make that big change just improve
>>> actual code. Of course real solutions are better then hacks.
>>> In fact "all" static attributes may be refactored out.
>>> However this vector is needed for updating scrolls and other stuffs, so
>>> not sure how it can be implemented properly ...
>>
>> class Screen2View
>> {
>> public:
>> int GetViewLine(int screenline);
>> private:
>> void RebuildVector();
>> vector<> m_screen2view;
>> };
>>
>> int GetViewLine(int screenline)
>> {
>> if (needsrebuild)
>> {
>> RebuildVector();
>> needrebuild = false;
>> }
>> return m_screen2View[screenline];
>> }
>>
>>
>> basically, you hide the vector completely from the outside. Not sure
>> why you think that updating scrollbars is somehow related to this?
> Because when I have not scroll bars and locator with delayed build it
> does not work. If you make it working it would be great.

Should work as of r21234.

Stefan

--
        ___
   oo // \\ "De Chelonian Mobile"
  (_,\/ \_/ \ TortoiseSVN
    \ \_/_\_/> The coolest Interface to (Sub)Version Control
    /_/ \_\ http://tortoisesvn.net

Re: [T-Merge Patch] Lock build to reduce build time

Author otik
Full name Oto BREZINA
Date 2011-04-28 03:29:01 PDT
Message On 2011-04-28 12:23, Stefan Küng wrote:
> On Thu, Apr 28, 2011 at 09:30, Oto BREZINA<otik@prin​tflow.eu> wrote:
>>
>> Sounds good to me, I did not want to make that big change just improve
>> actual code. Of course real solutions are better then hacks.
>> In fact "all" static attributes may be refactored out.
>> However this vector is needed for updating scrolls and other stuffs, so
>> not sure how it can be implemented properly ...
>
> class Screen2View
> {
> public:
> int GetViewLine(int screenline);
> private:
> void RebuildVector();
> vector<> m_screen2view;
> };
>
> int GetViewLine(int screenline)
> {
> if (needsrebuild)
> {
> RebuildVector();
> needrebuild = false;
> }
> return m_screen2View[screenline];
> }
>
>
> basically, you hide the vector completely from the outside. Not sure
> why you think that updating scrollbars is somehow related to this?
Because when I have not scroll bars and locator with delayed build it
does not work. If you make it working it would be great.
> Stefan

--
Oto BREZINA, Printflow s.r.o., EU

Re: [T-Merge Patch] Lock build to reduce build time

Author steveking
Full name Stefan Küng
Date 2011-04-28 03:24:05 PDT
Message On Thu, Apr 28, 2011 at 09:30, Oto BREZINA <otik at printflow dot eu> wrote:
> On 2011-04-28 08:53, Stefan Küng wrote:
>> On Wed, Apr 27, 2011 at 23:22, Oto BREZINA<otik@prin​tflow.eu>  wrote:
>>> On 2011-04-27 22:22, Oto BREZINA wrote:
>>>> second try
>>>>
>>>> It saves about 6  BuildAllScreen2ViewVector executions on start and
>>>> about 5 calls per reload.
>>>>
>>>> This is first approach for review (you may suggest better names) to
>>>> don't spend too much time with something wrong from base.
>>> Same with:
>>> Using array+enum instead of set of variables
>>> Better handling of update of locator
>> Haven't checked your patch yet (still in the office).
>> But I had an idea on how to deal with this without using
>> locks/counters/whatever:
>>
>> How about using a separate class for the screen/view vector. Access to
>> the vector is controlled by the class methods. That way, you can just
>> set a flag "rebuild necessary", and the vector is then rebuilt on the
>> first read access.
>> With such an implementation, the vector really is only rebuilt when necessary.
>>
>> Thoughts?
> Sounds good to me, I did not want to make that big change just improve
> actual code. Of course real solutions are better then hacks.
> In fact "all" static attributes may be refactored out.
> However this vector is needed for updating scrolls and other stuffs, so
> not sure how it can be implemented properly ...


class Screen2View
{
public:
  int GetViewLine(int screenline);
private:
 void RebuildVector();
 vector<> m_screen2view;
};

int GetViewLine(int screenline)
{
  if (needsrebuild)
  {
    RebuildVector();
    needrebuild = false;
  }
  return m_screen2View[screenline];
}


basically, you hide the vector completely from the outside. Not sure
why you think that updating scrollbars is somehow related to this?

Stefan

--
       ___
  oo  // \\      "De Chelonian Mobile"
 (_,\/ \_/ \     TortoiseSVN
   \ \_/_\_/>    The coolest Interface to (Sub)Version Control
   /_/   \_\     http://tortoisesvn.net

Re: [T-Merge Patch] Lock build to reduce build time

Author otik
Full name Oto BREZINA
Date 2011-04-28 00:30:14 PDT
Message On 2011-04-28 08:53, Stefan Küng wrote:
> On Wed, Apr 27, 2011 at 23:22, Oto BREZINA<otik@prin​tflow.eu> wrote:
>> On 2011-04-27 22:22, Oto BREZINA wrote:
>>> second try
>>>
>>> It saves about 6 BuildAllScreen2ViewVector executions on start and
>>> about 5 calls per reload.
>>>
>>> This is first approach for review (you may suggest better names) to
>>> don't spend too much time with something wrong from base.
>> Same with:
>> Using array+enum instead of set of variables
>> Better handling of update of locator
> Haven't checked your patch yet (still in the office).
> But I had an idea on how to deal with this without using
> locks/counters/whatever:
>
> How about using a separate class for the screen/view vector. Access to
> the vector is controlled by the class methods. That way, you can just
> set a flag "rebuild necessary", and the vector is then rebuilt on the
> first read access.
> With such an implementation, the vector really is only rebuilt when necessary.
>
> Thoughts?
Sounds good to me, I did not want to make that big change just improve
actual code. Of course real solutions are better then hacks.
In fact "all" static attributes may be refactored out.
However this vector is needed for updating scrolls and other stuffs, so
not sure how it can be implemented properly ...

> Stefan

--
Oto BREZINA, Printflow s.r.o., EU

Re: [T-Merge Patch] Lock build to reduce build time

Author steveking
Full name Stefan Küng
Date 2011-04-27 23:56:15 PDT
Message On Wed, Apr 27, 2011 at 23:22, Oto BREZINA <otik at printflow dot eu> wrote:
> On 2011-04-27 22:22, Oto BREZINA wrote:
>> second try
>>
>> It saves about 6  BuildAllScreen2ViewVector executions on start and
>> about 5 calls per reload.
>>
>> This is first approach for review (you may suggest better names) to
>> don't spend too much time with something wrong from base.
> Same with:
> Using array+enum instead of set of variables
> Better handling of update of locator

Haven't checked your patch yet (still in the office).
But I had an idea on how to deal with this without using
locks/counters/whatever:

How about using a separate class for the screen/view vector. Access to
the vector is controlled by the class methods. That way, you can just
set a flag "rebuild necessary", and the vector is then rebuilt on the
first read access.
With such an implementation, the vector really is only rebuilt when necessary.

Thoughts?

Stefan

--
       ___
  oo  // \\      "De Chelonian Mobile"
 (_,\/ \_/ \     TortoiseSVN
   \ \_/_\_/>    The coolest Interface to (Sub)Version Control
   /_/   \_\     http://tortoisesvn.net

Re: [T-Merge Patch] Lock build to reduce build time

Author otik
Full name Oto BREZINA
Date 2011-04-27 14:23:08 PDT
Message On 2011-04-27 22:22, Oto BREZINA wrote:
> second try
>
> It saves about 6 BuildAllScreen2ViewVector executions on start and
> about 5 calls per reload.
>
> This is first approach for review (you may suggest better names) to
> don't spend too much time with something wrong from base.
Same with:
Using array+enum instead of set of variables
Better handling of update of locator
--
Oto BREZINA, Printflow s.r.o., EU
Attachments

Re: [T-Merge Patch] Lock build to reduce build time

Author otik
Full name Oto BREZINA
Date 2011-04-27 13:22:37 PDT
Message second try

It saves about 6 BuildAllScreen2ViewVector executions on start and
about 6 calls per reload.

This is first approach for review (you may suggest better names) to
don't spend too much time with something wrong from base.

On 2011-04-27 09:21, Oto BREZINA wrote:
--
Oto BREZINA, Printflow s.r.o., EU
Attachments

[T-Merge Patch] Lock build to reduce build time

Author otik
Full name Oto BREZINA
Date 2011-04-27 00:21:16 PDT
Message --
Oto BREZINA, Printflow s.r.o., EU
Attachments
Messages per page: