Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5915 --- This review has been submitted with commit d634bc01c50e0a312a0f

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5914 --- This review has been submitted with commit a4871052d09eecd92866

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5910 --- Ship it! Hmm, indeed. I had another version of these patches in

Re: Review Request: Do not terminate threads

> On Aug. 21, 2011, 10:29 a.m., David Faure wrote: > > I think this patch is almost ready, but Dawit's comment seems right about > > an improvement that should be done to it: > > > > - Move all the preemptive lookup logic from the thread's run function to > > HostInfo::lookupHost. [No need to

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5875 --- I think this patch is almost ready, but Dawit's comment seems ri

Re: Review Request: Do not terminate threads

On Sunday 14 August 2011 23.05.41 Albert Astals Cid wrote: > #2 Your patch has several issues i mentioned there I noticed those too, and I wanted to just say that I'd trust David and Thiago on these concepts any day. Maybe we can just use the structure they suggested (and Albert coded) and move

Re: Review Request: Do not terminate threads

> On Aug. 12, 2011, 3:45 a.m., Dawit Alemayehu wrote: > > #1. Doesn't this approach have similar issues to the one that forced me to > > change the previous QtConcurrent::run based implementation ? That is, > > doesn't the use of a single thread expose this function to lookup requests > > back

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5654 --- #1. Doesn't this approach have similar issues to the one that fo

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ --- (Updated Aug. 11, 2011, 10:10 p.m.) Review request for kdelibs and Dawit A

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5653 --- kio/kio/hostinfo.cpp

Re: Review Request: Do not terminate threads

> On Aug. 11, 2011, 11:11 a.m., Thiago Macieira wrote: > > kio/kio/hostinfo.cpp, line 118 > > > > > > This class could be simplified to a simple struct. Yeah, but what's the benefit? > On Aug. 11, 2011, 11:11 a.m

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5636 --- kio/kio/hostinfo.cpp

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ --- (Updated Aug. 11, 2011, 10:34 a.m.) Review request for kdelibs and Dawit A

Re: Review Request: Do not terminate threads

> On Aug. 11, 2011, 9:41 a.m., David Faure wrote: > > kio/kio/hostinfo.cpp, line 236 > > > > > > Maybe move nameLookupThread as a member of hostInfoAgentPrivate, to > > avoid multiplying the global statics (and the

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ --- (Updated Aug. 11, 2011, 10:32 a.m.) Review request for kdelibs and Dawit A

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5623 --- Good job, this is tricky code indeed. Some comments below. kio

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ --- (Updated Aug. 11, 2011, 9:23 a.m.) Review request for kdelibs and Dawit Al

Review Request: Do not terminate threads

On Sat, Aug 6, 2011 at 7:46 AM, Albert Astals Cid wrote: > On Dijous 04 Agost 2011 15:28:49 Dawit A wrote: >> On Thu, Aug 4, 2011 at 5:31 AM, Albert Astals Cid wrote: >> >    This is an automatically generated e-mail. To reply, visit: >> > http://git.reviewboard.kde.org/r/102179/ >> > >> > On Aug

Re: Review Request: Do not terminate threads

On Dijous 04 Agost 2011 15:28:49 Dawit A wrote: > On Thu, Aug 4, 2011 at 5:31 AM, Albert Astals Cid wrote: > >This is an automatically generated e-mail. To reply, visit: > > http://git.reviewboard.kde.org/r/102179/ > > > > On August 4th, 2011, 3:19 a.m., *Dawit Alemayehu* wrote: > > > > I do

Re: Review Request: Do not terminate threads

On Thu, Aug 4, 2011 at 9:11 AM, Thiago Macieira wrote: >This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102179/ > > Ok, then there's something wrong. QHostInfo has had a blocking method since > Qt 4.0. > > Thiago, you must have forgotten but you

Re: Review Request: Do not terminate threads

On Thu, Aug 4, 2011 at 5:31 AM, Albert Astals Cid wrote: >This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102179/ > > On August 4th, 2011, 3:19 a.m., *Dawit Alemayehu* wrote: > > I do not like this patch for the very reason you stated. I do not w

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5396 --- That makes sense. Then we need only one thread, one that runs t

Re: Review Request: Do not terminate threads

> On Aug. 4, 2011, 1:11 p.m., Thiago Macieira wrote: > > Ok, then there's something wrong. QHostInfo has had a blocking method since > > Qt 4.0. Can not use it since we have a timeout parameter. Thus we either need the thread (to use the blocking method and forget about the thread when the tim

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5390 --- Ok, then there's something wrong. QHostInfo has had a blocking m

Re: Review Request: Do not terminate threads

> On Aug. 4, 2011, 7:14 a.m., Thiago Macieira wrote: > > QHostInfo is already threaded. This code is unnecessary today and should be > > removed. If we remove the thread we need to introduce a nested eventloop since that method is public and supposed to be blocking. Is that OK? - Albert --

Re: Review Request: Do not terminate threads

> On Aug. 4, 2011, 3:19 a.m., Dawit Alemayehu wrote: > > I do not like this patch for the very reason you stated. I do not want the > > mutex there either because it is rather expensive. As it stands we start a > > thread for each lookup right now which in of itself is already too > > expensiv

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5371 --- QHostInfo is already threaded. This code is unnecessary today an

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/#review5369 --- I do not like this patch for the very reason you stated. I do no

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ --- (Updated Aug. 2, 2011, 11:30 a.m.) Review request for kdelibs and Dawit Al

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ --- (Updated Aug. 2, 2011, 11:29 a.m.) Review request for kdelibs and Dawit Al

Re: Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ --- (Updated Aug. 2, 2011, 11:29 a.m.) Review request for kdelibs and Dawit Al

Review Request: Do not terminate threads

--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102179/ --- Review request for kdelibs and Dawit Alemayehu. Summary --- Each time