Cargo Distribution

Forum for technical discussions regarding development. If you have a general suggestion, problem or comment, please use one of the other forums.

Moderator: OpenTTD Developers

User avatar
ChillCore
Tycoon
Tycoon
Posts: 2658
Joined: 04 Oct 2008 23:05
Location: Lost in spaces

Re: Cargo Distribution

Post by ChillCore » 11 Apr 2009 12:10

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.
-- .- -.-- / - .... . / ..-. --- .-. -.-. . / -... . / .-- .. - .... / -.-- --- ..- .-.-.-
--- .... / -.-- . .- .... --..-- / .- -. -.. / .--. .-. .- .. ... . / - .... . / .-.. --- .-. -.. / ..-. --- .-. / .... . / --. .- ...- . / ..- ... / -.-. .... --- --- -.-. .... --- --- ... .-.-.- / ---... .--.

Playing with my patchpack? Ask questions on usage and report bugs in the correct thread first, please.
All included patches have been modified and are no longer 100% original.

-3s-
Engineer
Engineer
Posts: 12
Joined: 09 Apr 2009 19:26
Location: La Haye, NL

Re: Cargo Distribution

Post by -3s- » 11 Apr 2009 13:53

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).

User avatar
ChillCore
Tycoon
Tycoon
Posts: 2658
Joined: 04 Oct 2008 23:05
Location: Lost in spaces

Re: Cargo Distribution

Post by ChillCore » 11 Apr 2009 15:21

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.
-- .- -.-- / - .... . / ..-. --- .-. -.-. . / -... . / .-- .. - .... / -.-- --- ..- .-.-.-
--- .... / -.-- . .- .... --..-- / .- -. -.. / .--. .-. .- .. ... . / - .... . / .-.. --- .-. -.. / ..-. --- .-. / .... . / --. .- ...- . / ..- ... / -.-. .... --- --- -.-. .... --- --- ... .-.-.- / ---... .--.

Playing with my patchpack? Ask questions on usage and report bugs in the correct thread first, please.
All included patches have been modified and are no longer 100% original.

CrossSide
Engineer
Engineer
Posts: 68
Joined: 18 Apr 2008 04:36

Re: Cargo Distribution

Post by CrossSide » 11 Apr 2009 22:19

Sorry, I'm a noob at this. But when I do 'apply patch' with Tortoise nothing appears. Can someone help please! :)

DaleStan
TTDPatch Developer
TTDPatch Developer
Posts: 10285
Joined: 18 Feb 2004 03:06
Contact:

Re: Cargo Distribution

Post by DaleStan » 12 Apr 2009 00:53

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.
To get a good answer, ask a Smart Question. Similarly, if you want a bug fixed, write a Useful Bug Report. No TTDPatch crashlog? Then follow directions.
Projects: NFORenum (download) | PlaneSet (Website) | grfcodec (download) | grfdebug.log parser

CrossSide
Engineer
Engineer
Posts: 68
Joined: 18 Apr 2008 04:36

Re: Cargo Distribution

Post by CrossSide » 12 Apr 2009 11:54

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!
Last edited by CrossSide on 12 Apr 2009 12:44, edited 2 times in total.

Conditional Zenith
Chief Executive
Chief Executive
Posts: 697
Joined: 10 Jun 2003 00:19
Location: Australia

Re: Cargo Distribution

Post by Conditional Zenith » 12 Apr 2009 12:43

Could you please use 'directory' or 'folder' instead of 'map'. In English, map means something else.

CrossSide
Engineer
Engineer
Posts: 68
Joined: 18 Apr 2008 04:36

Re: Cargo Distribution

Post by CrossSide » 12 Apr 2009 12:45

Changed it, sorry my English isn't perfect ;)

User avatar
ChillCore
Tycoon
Tycoon
Posts: 2658
Joined: 04 Oct 2008 23:05
Location: Lost in spaces

Re: Cargo Distribution

Post by ChillCore » 12 Apr 2009 13:08

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.
-- .- -.-- / - .... . / ..-. --- .-. -.-. . / -... . / .-- .. - .... / -.-- --- ..- .-.-.-
--- .... / -.-- . .- .... --..-- / .- -. -.. / .--. .-. .- .. ... . / - .... . / .-.. --- .-. -.. / ..-. --- .-. / .... . / --. .- ...- . / ..- ... / -.-. .... --- --- -.-. .... --- --- ... .-.-.- / ---... .--.

Playing with my patchpack? Ask questions on usage and report bugs in the correct thread first, please.
All included patches have been modified and are no longer 100% original.

CrossSide
Engineer
Engineer
Posts: 68
Joined: 18 Apr 2008 04:36

Re: Cargo Distribution

Post by CrossSide » 12 Apr 2009 13:58

I tried to change it but I cannot copy/paste? Sorry, I'm really not good at this ;)

Alberth
OpenTTD Developer
OpenTTD Developer
Posts: 4746
Joined: 09 Sep 2007 05:03
Location: home

Re: Cargo Distribution

Post by Alberth » 12 Apr 2009 15:50

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 "+").

User avatar
fonso
President
President
Posts: 945
Joined: 13 Oct 2007 08:28

Re: Cargo Distribution

Post by fonso » 13 Apr 2009 11:23

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?
The guy on the picture is not me, it's Alonso.

User avatar
Zuu
OpenTTD Developer
OpenTTD Developer
Posts: 4553
Joined: 09 Jun 2003 18:21
Location: /home/sweden

Re: Cargo Distribution

Post by Zuu » 13 Apr 2009 11:41

Code: Select all

1/accuracy < epsilon
can be changed to

Code: Select all

1 < epsilon * accuracy
In pure math, these are equivivalent, but when using integers such changes can be of significant importance.
My OpenTTD contributions (AIs, Game Scripts, patches, OpenTTD Auto Updater, and some sprites)
Junctioneer (a traffic intersection simulator)

User avatar
XeryusTC
Tycoon
Tycoon
Posts: 15415
Joined: 02 May 2005 11:05
Skype: XeryusTC
Location: localhost

Re: Cargo Distribution

Post by XeryusTC » 13 Apr 2009 12:37

Using floats isn't a good idea :o the devs avoid them because they are known to cause desyncs and other nasty things. Almost everything can be calculated with integers too AFAIK.
Don't panic - My YouTube channel - Follow me on twitter (@XeryusTC) - Play Tribes: Ascend - Tired of Dropbox? Try SpiderOak (use this link and we both get 1GB extra space)
Image
OpenTTD: manual #openttdcoop: blog | wiki | public server | NewGRF pack | DevZone
Image Image Image Image Image Image Image

User avatar
fonso
President
President
Posts: 945
Joined: 13 Oct 2007 08:28

Re: Cargo Distribution

Post by fonso » 13 Apr 2009 12:49

Zuu wrote:

Code: Select all

1/accuracy < epsilon
can be changed to

Code: Select all

1 < epsilon * accuracy
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 :o 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.
The guy on the picture is not me, it's Alonso.

isk
Engineer
Engineer
Posts: 4
Joined: 05 Oct 2007 16:56

Re: Cargo Distribution

Post by isk » 13 Apr 2009 18:08

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 :)
Attachments
debug 2, 28th Feb 1950.sav
(105.33 KiB) Downloaded 93 times

User avatar
fonso
President
President
Posts: 945
Joined: 13 Oct 2007 08:28

Re: Cargo Distribution

Post by fonso » 13 Apr 2009 18:20

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.
The guy on the picture is not me, it's Alonso.

bokkie
Transport Coordinator
Transport Coordinator
Posts: 326
Joined: 19 Jan 2007 19:26

Re: Cargo Distribution

Post by bokkie » 13 Apr 2009 19:41

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 :)

MJS
Director
Director
Posts: 539
Joined: 28 Jul 2005 09:31

Re: Cargo Distribution

Post by MJS » 13 Apr 2009 19:49

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...

User avatar
ChillCore
Tycoon
Tycoon
Posts: 2658
Joined: 04 Oct 2008 23:05
Location: Lost in spaces

Re: Cargo Distribution

Post by ChillCore » 13 Apr 2009 20:21

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.
-- .- -.-- / - .... . / ..-. --- .-. -.-. . / -... . / .-- .. - .... / -.-- --- ..- .-.-.-
--- .... / -.-- . .- .... --..-- / .- -. -.. / .--. .-. .- .. ... . / - .... . / .-.. --- .-. -.. / ..-. --- .-. / .... . / --. .- ...- . / ..- ... / -.-. .... --- --- -.-. .... --- --- ... .-.-.- / ---... .--.

Playing with my patchpack? Ask questions on usage and report bugs in the correct thread first, please.
All included patches have been modified and are no longer 100% original.

Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: ino and 2 guests