Log for on 16th February 2014:
Times are UTC Toggle Colours
09:14:13  *** Alberth has joined
09:14:13  *** ChanServ sets mode: +v Alberth
11:16:32  <fonsinchen> I'm going to rewrite the smallstack without using Pool. It's impossible to verify that it's used correctly like this.
11:17:19  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26341 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
11:31:18  <planetmaker> I think we should also allow full svn check-outs and not fail version checks when building tags (releases) from those:
11:36:30  <fonsinchen> Can we be sure that those are enough '../'?
11:37:37  <planetmaker> I only add a check for yet another level higher
11:37:54  <planetmaker> ../ in / is the same as /
11:40:48  <fonsinchen> It will fail if the user puts a partial non-svn checkout of openttd into an unrelated svn checkout. That's a fairly pathetic case, thouh.
11:41:49  <LordAro> i think that goes into the realms of "stupid user is stupid"
11:42:06  <planetmaker> that already can fail
11:42:17  <planetmaker> but that's indeed a rather pathetic case, I think
11:42:41  <planetmaker> it's more that I could not build a release version from my existing svn checkout
11:43:41  <planetmaker> though a complete svn checkout is also somewhat pathetic, I think there are more users who use that (even somewhat legitimately) than those who have a .svn two levels higher than their openttd svn checkout
11:43:44  <fonsinchen> But now it can fail on two levels up from openttd's root dir. We could (semi-) fix that by checking for something specific to openttd in that directory.
11:44:21  <fonsinchen> Whatever, if it helps, it's OK for me.
11:45:05  <planetmaker> well, it's still guarded by the other conditions which still must be fulfilled: && [ -n "`LC_ALL=C svn info $ROOT_DIR/.. | grep '^URL:.*tags$'`" ]
11:45:29  <planetmaker> thus you must be in a dir which has 'tags' in the path
11:45:41  <planetmaker> which is pretty openttd-specific for the releases
11:50:46  <planetmaker> I actually would want to know from Rubidium why a "full svn checkout (is) discouraged"
11:51:02  <planetmaker> except that there's of course little reason to have that for most people
12:32:25  *** Supercheese has quit IRC
12:32:57  *** Supercheese has joined
12:32:57  *** ChanServ sets mode: +v Supercheese
12:36:55  *** frosch123 has joined
12:36:55  *** ChanServ sets mode: +v frosch123
13:05:23  *** Zuu has joined
13:05:23  *** ChanServ sets mode: +v Zuu
13:05:42  *** Zuu has left
13:07:34  <Rubidium> planetmaker: you really need the 131 other non-1.3.3/1.4.0-beta4 subversion checkouts? Of which probably half don't even compile?
13:07:34  <Rubidium> generally, if you want to check whether something was backported, the stable branch is better anyhow
13:08:40  <planetmaker> "need" is relative. But yes, I have many of those and I sometimes use one or the other to check something
13:08:51  <planetmaker> difficult if it doesn't compile, though, indeed
13:11:28  * Rubidium only has the .0 release and stable branches of anything < 1.3.3
13:12:09  <Alberth> just download the source distribution?
13:13:20  <planetmaker> ok, is there any problem other than "I don't use it"?
13:40:40  <planetmaker> but I really don't get why it's now a bad idea, Rubidium
14:05:37  <fonsinchen> In fact the smallstack leaks. In its destructor it only deletes the first item beyond the head, not the full stack.
14:06:10  <planetmaker> oi.
14:07:52  <fonsinchen> You need a pretty complex game with lots of nested conditional orders for this to be a problem though.
14:10:48  <planetmaker> hm, this behaviour is new to me... is there a max number of open windows?
14:12:19  <planetmaker> I cannot open more than 9 concurrent vehicle windows
14:12:39  <fonsinchen> That's not new. I've run into that quite a few times.
14:14:47  <planetmaker> so if the conditional orders are required for a desync based on the smallstack leak, it's not the reason in the game we had running on welcome server
14:14:57  <planetmaker> I didn't find a train with conditional orders in any company
14:15:59  <fonsinchen> I can't imagine it causes a desync. It could cause a crash when the 64k smallstack items run out.
14:16:00  <frosch123> planetmaker: there is an advanced setting to limit the amount of non-sticky windows
14:17:37  <planetmaker> wow, another setting I can't remember :D
14:17:58  <frosch123> before 0.7 or so there was a fixed amount of windows
14:18:16  <frosch123> it was then changed to a configurable soft limit
14:18:58  <planetmaker> that's long ago :)
14:20:49  <frosch123> fonsinchen: does the order of items in the pool matter somehow?
14:21:26  <frosch123> if the server has more than the clients, the clients will assign different ids than the server
14:21:59  <fonsinchen> I don't quite get that ...
14:22:24  <fonsinchen> The server can assign different IDs than the clients without that being a desync?
14:23:15  <fonsinchen> But the order of items doesn't matter. It's basically a linked list with custom new/delete.
14:24:23  <frosch123> ah, i guess even the leaked items will still be saved, so also exist on the clients after load
14:24:42  <frosch123> or is that pool not saved?
14:26:23  <fonsinchen> It's not saved
14:26:32  <fonsinchen> At least I hope so
14:26:48  <fonsinchen> I'm currently rewriting it to not use a pool at all.
15:41:29  <fonsinchen> ...
15:41:43  <fonsinchen>
15:41:54  <fonsinchen> To fix the smallstack problem
15:42:06  <frosch123> he, i just thought earlier to write a mutex lockguard
15:42:56  <frosch123> rename ThreadMutexLocker to TheadMutexLockGuard? afaik "lock guard" is the common name for that
15:43:24  <frosch123> yeah, there is a std::lock_guard in c++11
15:45:40  <fonsinchen> Qt has MutexLocker
15:45:49  <fonsinchen> QMutexLocker that is
15:46:03  <frosch123> ok, i never really used qt
15:46:50  <LordAro> ew, Qt
15:46:56  <LordAro> woo, c++11
15:46:58  <LordAro> :)
15:47:49  <frosch123> do you have a highlight on c++11 ?
15:48:07  <fonsinchen> I think he has a highlight on Qt :)
15:48:55  <frosch123> really? but qt rarely refers to actual girls
15:49:30  <fonsinchen> That's why he says "ew" everytime he sees it.
15:52:23  <Rubidium> planetmaker: what percentage of users will need a full checkout? They generally only need one version, so generally... it's discouraged
15:52:52  <frosch123> fonsinchen: to nitpick: the smallstack is only "thread-safe", not "reentrant"
15:53:14  <fonsinchen> thread-safe is more than reentrant
15:53:15  <planetmaker> Rubidium, yes, discourage. But have it fail to deliver a usable source?
15:53:30  <frosch123> what? reentrant is more than thread-safe
15:53:32  <fonsinchen> reentrant means you can use different objects of it in different threads
15:53:34  <Rubidium> planetmaker: that's because subversion changed its format
15:53:43  <Rubidium> ... of working copies
15:53:46  <fonsinchen> thread-safe means you can use the same object concurrently
15:54:04  <frosch123> reentrant means you can interrupt any thread at any time, and use the thing
15:54:21  <Rubidium> planetmaker: and when I made the 'fix' I genuinely believed I was more or less the only one needing it for the tags as you can see in the configure comment
15:55:47  <planetmaker> yes, I do see that. And I agree the amount of people who have some reason to use it is about as limited as the people in this channel, rather less. Yet...
15:56:59  <planetmaker> ... does it hurt anywhere?
15:57:17  <planetmaker> currently, if someone got full svn, s/he will need to checkout parts of it again
15:57:51  <Rubidium> for what it's worth, I have a "shallow" / mostly "empty" checkout of tags, so I only needed it to look one path higher. From that I usually remove "old" tags after a few weeks.
15:58:19  <Rubidium> the branches are just separate checkouts
15:58:23  <fonsinchen> reentrancy is kind of a convoluted explanation of the concept. You can of course interrupt any call to any method of SmallStack and later continue it if you don't modify the same smallstack in between.
15:58:45  <fonsinchen> thread-safe means you can also do that while using the same smallstack.
15:59:06  <planetmaker> well, I have one svn, where I usually only update trunk. or in the tags dir what I need
15:59:10  <fonsinchen> So my implementation is reentrant, but not thread-safe.
15:59:42  <frosch123> i thought the reverse :p i learned "mutexes make code thread-safe, but never reentrant"
15:59:52  <Rubidium> planetmaker: feel free to add an extra level, though be sure to not copy the "(tags)" comment as that won't be right
16:00:22  <fonsinchen> Well, I made the calls to the underlying pool thread safe with mutexes
16:00:26  <frosch123> mutexes protect other threads from interfering, but not the same thread from interfering with itself
16:00:41  <frosch123> but well, does not matter
16:02:02  <LordAro> fonsinchen, frosch123: D:
16:02:07  <LordAro> but no, i've just been lurking
16:07:48  <frosch123> +		if (index < Tmax_size) { <- shouldn't that be an assert in Create?
16:08:09  <frosch123> Push seems to just ignore the push when the pool is filled
16:08:17  <fonsinchen> See the documentation of Push
16:08:21  <frosch123> hmm, yeah :p
16:08:46  <fonsinchen> it is in fact possible to create pathetic cases with more than 64k branches in conditional orders.
16:08:58  <fonsinchen> I've decided to ignore that then.
16:11:26  <fonsinchen> You're probably right in that I shouldn't call this reentrant
16:12:22  <fonsinchen> The definitions of reentrancy differ, though:
16:14:32  <Rubidium> as long as the function doesn't have an internal state it's reentrant by definition, right?
16:14:42  <Rubidium> e.g. max/min/abs is reentrant
16:15:14  <frosch123> "Note: Terminology in the multithreading domain isn't entirely standardized. POSIX uses definitions of reentrant and thread-safe that are somewhat different for its C APIs. When using other object-oriented C++ class libraries with Qt, be sure the definitions are understood." <- from that page :)
16:15:47  <frosch123> so, qt's definiton of reentrant is very different to the posix one
16:16:43  <fonsinchen> apparently
16:16:57  <fonsinchen> Do we have a better name for what they call reentrant?
16:17:35  <Rubidium> IMHO reentrant functions are functions that do not modify (global) state
16:18:04  <frosch123> that's the posix definition
16:18:36  <frosch123> hmm, kind of both :p
16:18:39  <Rubidium> e.g. anything that touches errno isn't reentrant, since if you call it from two threads... even when they are utterly different (e.g. writing file A and reading file B), because upon error you wouldn't know what error you actually got
16:20:21  <frosch123> fonsinchen: diff looks fine
16:24:44  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26342 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
16:25:19  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26343 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
16:39:44  <planetmaker> <-- any objects? That makes sense to me and I didn't find anything which is negatively impacted
16:39:51  <planetmaker> *objections
16:43:42  <frosch123> i guess trees are often enough on shores to justify that opimisation
17:03:59  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26344 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
17:29:04  <fonsinchen> YES. I finally can reproduce FS#5898
17:29:17  <planetmaker> <-- makes sense to me, too
17:33:26  <frosch123> planetmaker: keep the stuff in InvalidateDimensionCache in the same order as in the defintion and constructions below?
17:33:33  <frosch123> that makes it easier to see that they are complete
17:34:39  <frosch123> it looks like whenever NWidgetLeaf::InvalidateDimensionCache(); is called, also NWidgetScrollbar::InvalidateDimensionCache(); is called
17:34:55  <frosch123> so, only calling one is likely wrong
17:35:02  <frosch123> is there maybe a more generic method to call?
17:35:15  <planetmaker> same order definitely makes sense
17:35:41  <frosch123> isn't ReInitAllWindows called somewhere?
17:35:53  <frosch123> if the sprite sizes really change, then reinit should be called
17:36:11  <planetmaker> the bug is real, though
17:36:21  <frosch123> but well, i cannot survey all call paths
17:36:22  <planetmaker> that sprite's width doesn't change
17:36:32  <planetmaker> ok, I shall look
17:36:36  <frosch123> that does not say that the fix is correct :p
17:36:49  <frosch123> the hunk in widget.cpp is ceratinly correct
17:37:04  <frosch123> i would claim that gfxinit is incorrect, though i do not know whether just calling the scrollbar thingie is enough
17:37:12  <frosch123> or whtehr it should be reinitallwindiows
17:37:23  <frosch123> or whether calling the latter would cause more troubles
17:38:50  <frosch123> anyway, it's the same patching place as fsä5867
17:39:14  <frosch123> i don't think that adding random functions to that place is fixing anything in long term
17:40:05  <planetmaker> hm
17:42:30  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26345 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
17:45:25  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26346 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
18:37:01  <fonsinchen> for FS#5898
18:37:41  <fonsinchen> Basically I'm moving the thread handling into the jobs because when a saveload fails it nulls all pointers and the schedule won't know its jobs anymore
18:38:20  <fonsinchen> However, we can still get hold of them on pool clean later, so that's no real problem.
18:40:42  <frosch123> looks fine to me
18:43:10  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26347 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
18:45:30  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26348 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
19:41:36  <frosch123> hmm, we use SendSignal and WaitForSignal with posix behaviour
19:42:03  <frosch123> but thread_os2 and thread_win32 actually do not provide the posix behaviour for WaitForSignal
19:42:26  <frosch123> the signal waiting and critical section entering/leaving is not atomic
19:48:55  <planetmaker> <-- hm... I'd have thought it might be fixed by invalidating also order windows...
19:52:58  <frosch123> you have to reinit or close them or something
19:53:26  <frosch123> the order window uses a different window_desc and widget tree depending on owned/competitor
19:53:33  <frosch123> it's the only window which does that :p
19:54:16  <frosch123> you either have to remove that magic from the order window by using nestedwidgetstacks
19:54:32  <frosch123> or you have to close all order windows, which may look weird :p
19:55:59  <planetmaker> yeah... and will quickly give the next bug report :P
19:57:05  <frosch123> oh, the win32 waitforsignal is actually fine
20:02:58  <frosch123> <- fixes the issues reported by helgrind with sdl
20:03:31  <frosch123> there were some weird assumptions which VideoDriver_SDL methods were called with which mutexes already acquired
20:03:51  <frosch123> this fixes it by making the mutex allowing recursive locking, and then always acquireing them
20:04:17  <frosch123> the os/2 and windows code is not tested whether it compiles :p
21:12:21  * Rubidium wonders whether OS/2 still compiles at all; guess you'd need to talk to orudge for that
21:14:08  <frosch123> iirc he some 1.4 beta
21:14:14  <frosch123> +compiled
21:14:25  <frosch123> but morphos looks weird
21:14:31  <frosch123> there is a thread class for it, but no mutex class
21:14:51  <planetmaker> who added / maintained morphos anyway?
21:14:55  <frosch123> tokai
21:15:16  <planetmaker> hm, saw him online the other day
21:15:23  <frosch123> but it stopped compiling around 0.6 when more c++ stuff got used
21:15:38  <frosch123> morphos had no newer compiler than gcc 2.95 for years
21:19:08  <Rubidium> frosch123: might be of interest for you
21:19:49  <frosch123> how boring :p
21:19:55  <frosch123> but thanks :)
21:20:03  <frosch123> saves a few iterations on the farm
21:20:44  <Rubidium> adding recursive_count removes all but two
21:21:06  <frosch123> yeah, need to add the default param also in the derived classes
21:32:57  *** Alberth has left
21:37:13  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26349 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
21:43:15  *** linuxman has joined
21:47:00  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26350 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
21:57:23  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r26351 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
21:57:53  <frosch123> now we only need to figure out whether the same applies to allegro, osx, windows, and what not :p
21:58:16  <LordAro> suspect you'll soon find out
21:59:10  <frosch123> planetmaker: btw. fs#5900 also one existed for sdl, because VideoDriver_SDL::ChangeResolution missed that mutex
21:59:23  <frosch123> but i have no idea wheather osx uses multithreaded drawing at all
21:59:36  <frosch123> LordAro: i had a task for you, but i do not remember what it was
21:59:51  <LordAro> :o
22:00:05  <LordAro> i know that i never finished the news item crashlog patch
22:00:18  <LordAro> that's pretty much the last time i did :(
22:00:35  <frosch123> well, there was also the "load heightmap/start scenario does not allow to view readme"
22:00:42  <frosch123> but i had another one a few days ago :p
22:03:43  *** frosch123 has quit IRC
23:03:05  *** linuxman has quit IRC

Powered by YARRSTE version: svn-trunk