Page 4 of 154
Re: Cargo Distribution
Posted: 11 Apr 2009 12:10
by ChillCore
Same slowdown here.
Winxp, MinGW-MSYS.
After some testing the problem seems to be with mailhandling.
If you leave mail unhandled -> no slowdowns.
Mind I only tested the slowdown and did not have a look at the source other than changing the patch to svn.
For testing i used a savegame.
hope this helps a bit.
Re: Cargo Distribution
Posted: 11 Apr 2009 13:53
by -3s-
ChillCore wrote:Same slowdown here.
Winxp, MinGW-MSYS.
After some testing the problem seems to be with mailhandling.
If you leave mail unhandled -> no slowdowns.
Mind I only tested the slowdown and did not have a look at the source other than changing the patch to svn.
For testing i used a savegame.
hope this helps a bit.
I tried it again, with mail unhandled (just like ChillCore). The same problem occurs again (even when I only let passengers have symetric handling).
I can also better pinpoint the moment the problem occurs. I let two buses drive from A->B and the other from A->C. No problems when they arrive at station A for the first time. When they arrive for the first time at B & C, then OpenTTD suddenly consumes 99% CPU. I have opened the stations GUI for stations A, B & C and clicked on the passengers (so I get more details).
Re: Cargo Distribution
Posted: 11 Apr 2009 15:21
by ChillCore
I really tried only changing one value at a time...
*blushing smiley here*
I tried that again with a clean cfg.
I left everything like set by the patch and reduced the accuracy setting to 1.
That did the trick, not mail.
Even when set to 8 the slowdown is significant.
sorry again for that false bit of information.
Re: Cargo Distribution
Posted: 11 Apr 2009 22:19
by CrossSide
Sorry, I'm a noob at this. But when I do 'apply patch' with Tortoise nothing appears. Can someone help please!

Re: Cargo Distribution
Posted: 12 Apr 2009 00:53
by DaleStan
What evidence do you have that nothing happened?
IOW, list exactly the steps you took, exactly the messages that the computer spat out at you, exactly the results you observed, and exactly the results you expected.
Re: Cargo Distribution
Posted: 12 Apr 2009 11:54
by CrossSide
1. I downloaded the diff. file from the openings post.
2. I created a folder and downloaded the files from svn.openttd.org with tortoise
3. Then I right clicked on that folder and choosed 'apply patch' and I clicked on the Cargodist.diff
4. Tortoise opened but then nothing happened...
Thanks for the help already!
Re: Cargo Distribution
Posted: 12 Apr 2009 12:43
by Conditional Zenith
Could you please use 'directory' or 'folder' instead of 'map'. In English, map means something else.
Re: Cargo Distribution
Posted: 12 Apr 2009 12:45
by CrossSide
Changed it, sorry my English isn't perfect

Re: Cargo Distribution
Posted: 12 Apr 2009 13:08
by ChillCore
4. Tortoise opened but then nothing happened...
You need to change the file in various places like this.
From:
diff --git a/source.list b/source.list
index 452a2c3..3431c2e 100644
--- a/source.list
+++ b/source.list
To:
Index: source.list
===================================================================
--- source.list (revision 16003)
+++ source.list (working copy)
For the new files you do this:
Index: src/linkgraph.cpp
===================================================================
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ src/linkgraph.cpp (working copy)
I tried pulling a diff for turtoise but the new files are not included in the created patch.
The patch i have for turtoise is the first 16003.
If you do not mind doing the upgrade to cargodist_r16003_2.diff yourself manually, i could post that one in the meantime.
Re: Cargo Distribution
Posted: 12 Apr 2009 13:58
by CrossSide
I tried to change it but I cannot copy/paste? Sorry, I'm really not good at this

Re: Cargo Distribution
Posted: 12 Apr 2009 15:50
by Alberth
Sure you can, a patch file is human readable. It is just a lot of work.
Lines starting with +++ or --- state which file is being handled.
Lines starting with @@ give line numbers
Lines starting with " " (a space) as first character are context, and you should find them exactly in your source file.
Lines starting with "-" as first character are to be deleted from the source file.
Lines starting with "+" as first character are to be copied at the appropiate place into the source file (without the leading "+").
Re: Cargo Distribution
Posted: 13 Apr 2009 11:23
by fonso
Thanks for the testing and the savegames. I think I can make some sense of it. So, to sum it up. Please correct me where I am wrong:
- The freezing occurs with builds on Win XP with mingw, possibly only those made with BuildOTTD (which generally uses gcc 3.4).
- It is possible to reduce the accuracy to 1, which shouldn't be. 1 for accuracy should yield fairly random results, if any. 2 is a meaningful minimum.
- When reducing accuracy to very small numbers the freezing problem is less likely to occur.
I guess the freezing is caused by another float<->integer conversion problem. Possibly with some compilers some cast is interpreted differently. Especially interesting in this context is that ChillCore claims the problem completely goes away with accuracy == 1. For the actual calculation I set a value epsilon to "1 / accuracy". Now 1/1 = 1 and 0 < 1/n < 1 for n > 1. If cast incorrectly epsilon will always be 0 if accuracy > 1. epsilon == 0 leads to an infinite loop in a different place which is an explanation for the freeze.
So, if you have a build and some time around, please try with accuracy == 1 and tell me if it still freezes.
Of course, if I can identify the problem to be related to my usage of floats, it will be obsolete as soon as the replacement for the MCF solver is ready.
TrueBrain wrote:Ever considered using a Mercurial or Git repository for your code? Easier to track, and easier for us to follow

I have a git repository here, but no easy way to publish it. Any ideas?
Re: Cargo Distribution
Posted: 13 Apr 2009 11:41
by Zuu
can be changed to
In pure math, these are equivivalent, but when using integers such changes can be of significant importance.
Re: Cargo Distribution
Posted: 13 Apr 2009 12:37
by XeryusTC
Using floats isn't a good idea

the devs avoid them because they are known to cause desyncs and other nasty things. Almost everything can be calculated with integers too AFAIK.
Re: Cargo Distribution
Posted: 13 Apr 2009 12:49
by fonso
Zuu wrote:
can be changed to
In pure math, these are equivivalent, but when using integers such changes can be of significant importance.
I know.
XeryusTC wrote:Using floats isn't a good idea

the devs avoid them because they are known to cause desyncs and other nasty things. Almost everything can be calculated with integers too AFAIK.
I know.
Did you read what I wrote on Thursday? Whatever - in order to find out *why* it freezes after a month I leave the float calculations in for now and test with them. Otherwise I might create a situation where the problem is less obvious but still there. Thus it would be harder to debug than the current situation. I'm not completely sure that the float calculations are causing the problem, it's only a hypothesis for now. So, if you'd like to help me, please try with accuracy == 1. I will change the MCF solver to only use integer calculations as soon as the freezing problem is solved.
Re: Cargo Distribution
Posted: 13 Apr 2009 18:08
by isk
Hi,
I've compiled your patch applied to r160003 with MSVC on WinXP and I think I've found a bug: the game asserts on
Code: Select all
template<class ANNOTATION>
void MultiCommodityFlow::Dijkstra(NodeID from, PathVector & paths) {
...
annos.erase(dest); //line 124 in mfc.cpp
...
}
with "invalid operator<"
You can reproduce it building two stations and a bus (I haven't tried any other type of transport) connecting them (A->B). The assertion will pop up some time after the bus reaches both bus stops for the first time (probably on first graph recalculation).
I had some time and started debugging your code and I think I've found the cause. The
Code: Select all
bool DistanceAnnotation::comp::operator()
can't be used as predicate because it's not strict when x == y as in that case it returns true. I've changed it to
Code: Select all
bool DistanceAnnotation::comp::operator()(const DistanceAnnotation * x, const DistanceAnnotation * y) const {
if(x == y)
return false;
return !greater(x->GetAnnotation(), y->GetAnnotation(), x, y);
}
and the problem was gone. Also after confirming that I can build one route and keep playing I've created a second line B->C with one bus and tried to reproduce the "freeze" bug, but it hasn't occurred (let it run on fast forward for more then 2 game years).
I've attached a save, the assert (or freeze if it's the same problem) should appear on 7th March 1950.
I hope these informations will help you to finish this great patch

Re: Cargo Distribution
Posted: 13 Apr 2009 18:20
by fonso
Thanks isk. We have just duplicated efforts. Actually I spent half the afternoon installing stuff into the BuildOTTD-supplied mingw in order to reproduce the problem and then another hour to find out how to interrupt the program manually to examine the stack, but finally I have managed to ... and found out that the freeze is caused by exactly the same effect. But it's still nice to see people care about this patch.
I'll post a fixed all-in-one version in the first post.
Re: Cargo Distribution
Posted: 13 Apr 2009 19:41
by bokkie
fonso wrote:But it's still nice to see people care about this patch.
Probably lots of people do just like me, but they lurk most of the time

Re: Cargo Distribution
Posted: 13 Apr 2009 19:49
by MJS
Don't worry that not enough people care - lots of us do, but for most (including me) the technical discussion is way over our head... And for some of us just a .patch is not enough; if anyone (not fonso, he has other things to do

) has a Windows .exe, that'd be handy...
Re: Cargo Distribution
Posted: 13 Apr 2009 20:21
by ChillCore
MJS wrote:
Don't worry that not enough people care - lots of us do, but for most (including me) the technical discussion is way over our head... And for some of us just a .patch is not enough; if anyone (not fonso, he has other things to do ) has a Windows .exe, that'd be handy...
The problem with posting buggy binaries is you can not change anything.
If you compile yourself that is another story.
@fonso:
I am going to test your patch and i will post a patch for turtoise.
If you think the patch is stable enough I will post my binaries too,
if that is okay with you that is.