Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disabled Channels Setting invalidates the network on too many channels #1510

Closed
thatsIch opened this issue May 25, 2015 · 35 comments
Closed
Labels
bug Self explanatory?

Comments

@thatsIch
Copy link
Member

collection entry

@thatsIch thatsIch added bug Self explanatory? awaiting-feedback Waiting on feedback from the author. Issue will be closed after 7 days of inactivity. labels May 25, 2015
@yueh
Copy link
Member

yueh commented May 26, 2015

I am still not convinced that disabling channels has any bug in this case.
If I build a network with AE2 alone and way larger than the usual ones posted here, it does not stop working in any way.

Obligatory screenshots:
build
network

Even when doubling the size again it will not affect anything.
Besides something dropping the FPS (I am suspecting the CGAs and their particles)

@Lyokomzm
Copy link

We have since re-activated channels on our server, however I will try to replicate it myself later this week.

I would like to note, we previously only used ME Glass Cable (various colors) and had Dense cables disabled also, as without channels they weren't needed.

@ghost
Copy link

ghost commented May 26, 2015

I use the normal ME Glass Cable trough my whole base. If you would like to, I can send the server config to you. So you can test it with that config.

@thatsIch
Copy link
Member Author

else blame chunks..

@Clasbyte
Copy link

Clasbyte commented Jun 9, 2015

Salve,

we have the same problem on our server. I turned channels off for convenience and it worked just fine till i hit a threshold.
We are running a FTB Infinity Server, the network that is currently in use is a lot smaller then what
yueh build for an test.
But we did use a lot of ME Conduits from Ender IO, i did remove some of the conduits to see if that gets me under the unseen threshold, that did not work. The only thing that allows me to set an new interface,export/import/storage bus et cetera, is to remove one of the external interaction units ME has.

Screenshot:
2015-06-09_18 35 26
2015-06-09_18 35 36

@thatsIch
Copy link
Member Author

thatsIch commented Jun 9, 2015

Please try with the latest build and report back,
yes I know the typical server answer,
but someday FTB will update their modpacks (~1-2 weeks or are even updated)

@Clasbyte
Copy link

Latest stable or beta build?

Edit.

Used rv2.stable build 6 no change the next terminal attached to the system has a rose light, the rest stays blue but the network stops working.

@thatsIch
Copy link
Member Author

Can you clarify what you meant with

external interaction units ME has

@Clasbyte
Copy link

Terminals of all kinds including the thaumcraft added one's, interfaces, import/export bus, storage bus.

@Clasbyte
Copy link

Found something new, i used some dense cable's to see if the problem is with the standard glass cable.
2015-06-12_19 49 05
2015-06-12_19 49 18
I removed some interface and some import bus and export bus entities, and saw that i got -20/32 channels on the dense cable, if i attach an terminal or a interface/bus of any kind, the channels i see on the dense cable go towards 0, the network stops working when it reach's 0/32 channels.

@thatsIch
Copy link
Member Author

uh, overflow? yuck..

@Clasbyte
Copy link

Found some more info and maybe a work around.
in the AppliedEnergistics2.cfg in the FTB infinity pack can the following entry be found

automation {
I:formationPlaneEntityLimit=128
}

If i set it to 256, my network still crashed when i reach 0/32 but as soon i attache one more entity i goes to 1/32 and starts functioning again.

@thatsIch
Copy link
Member Author

Just noticed, how do you have Dense Cables, when you have channels disabled?

@Clasbyte
Copy link

I do not know, i can build the standard the covered and the dense one, controller and smart are disabled, would be cool if they are active even if the channels are disabled so you can pre build an setup if you want to switch back to channels.

@thatsIch
Copy link
Member Author

#1474

@thatsIch
Copy link
Member Author

So after my assumption is, that it breaks after using 255 channels.. or on 256th channel usage, because it is stored in a byte

@thatsIch
Copy link
Member Author

second assumption is, that it is not breaking, but the system think that you are using none or just one channel etc..

@thatsIch
Copy link
Member Author

Do you per chance have the ability to spawn in a creative only DebugCard via the config option B:UnsupportedDeveloperTools by setting it from false to true?

@Clasbyte
Copy link

I can do that, what information do you need ?

@Clasbyte
Copy link

Looks like i spoke to soon, i changed the setting restarted the server and now i get an time out when i try to connect.

@Clasbyte
Copy link

Ok got it.

2015-06-15_20 26 00

@thatsIch
Copy link
Member Author

Yea, you see the Node Channels section?

I just wanted a clear knowledge how many are actually there, WAILA uses some other info.

So my guess is, that if your network reaches 256, it will start to break down. Keep away from that number atm (from 8 to 256 should be an improvement, right?)

@yueh
Copy link
Member

yueh commented Jun 15, 2015

This does not really make sense, as my example with a couple of thousand nodes is still working.
Maybe only related to dense cables? (Which should be deactivated)

@Lyokomzm
Copy link

I can confirm that this problem still occurs on a server with dense cables disabled.

Also, dense cables is a separate config from enabling channels.

@Clasbyte
Copy link

Ah i can go over that number now, as i mentioned before.

automation {
I:formationPlaneEntityLimit=128
}

If i set it to 256, my network still crashed when i reach 0/32 but as soon i attache one more entity i goes to 1/32 and starts functioning again.

I will later attach over 256 items to test it again. Also yueh has a very good point his network is functioning with a lot more stuff.

@yueh
Copy link
Member

yueh commented Jun 15, 2015

You think changing a completely random and unrelated config setting will change anything?

@thatsIch thatsIch removed the awaiting-feedback Waiting on feedback from the author. Issue will be closed after 7 days of inactivity. label Jun 16, 2015
@thatsIch
Copy link
Member Author

I can confirm, that it breaks at exactly 256 nodes on a single network, based on cables spread in all 4 directions, just using storage monitors. After adding more, it starts to work again, it will probably start to fail at a multiple of 256. I wonder if there is something like a 0 check somewhere, preventing it from working, since it is based on a byte value, generally with the setting off, it will not happen.

ScreenShot:
http://puu.sh/iqPdz/94e267da5a.png

Save File:
http://puu.sh/iqORO/38b0af0e79.zip

@yueh
Copy link
Member

yueh commented Jun 16, 2015

Most likely GridNode.java#L388 and GridNode.java#L362
It will overflow to 0 every 256 channels.

We only use 2*1 byte of the int to encode the channels, so we could change it to 2*2 bytes.
But this is only a workaround and will then fail every 65536 nodes.

I doubt it is possible to correctly implement disabled channels currently without reimplementing alternative versions for the most important classes like GridNode or the whole pathing.

@thatsIch
Copy link
Member Author

Using the correct type would help a lot probably. Storing information in specific parts of ints and longs is ok, as you use it to transfer information.

I think hot-fixing it to 64k is probably ok for now. This is exponentially harder to hit, since you require certain amounts of mods and machines to even get that high.

@yueh
Copy link
Member

yueh commented Jun 16, 2015

It basically creates a copy of the used channels to detect if something actually has changed.
I can also understand ot reuse it as it normally will not exceed 32 channels so it can actually save bit of memory as GridNode should be really small object.

But then storing the security key as long is probably wasting more memory than just splitting it into currentUsedChannels and previousUsedChannels and fully utilizing an int.
Of course there is always an theoretical limit with int or long, but reaching that with channels will have killed minecraft way earlier. But for real "disabled channels" it should just depend on being powered.

Splitting it into two would be even a (very) small performance improvement as it no longer needs a bit operation everytime something access the data.

yueh added a commit to yueh/Applied-Energistics-2 that referenced this issue Jun 16, 2015
Previously it did encode the current and previous used channels into the
same as well as mask it with 0xFF. Which lead to an overflow every 256
gridnodes requiring a channel. This will not happen at > 2^31

Also removes the need to bitshift them for every access.

Fixes AppliedEnergistics#1510
yueh added a commit to yueh/Applied-Energistics-2 that referenced this issue Jun 16, 2015
Previously it did encode the current and previous used channels into the
same as well as mask it with 0xFF. Which lead to an overflow every 256
gridnodes requiring a channel. This will not happen at > 2^31

Also removes the need to bitshift them for every access.

Fixes AppliedEnergistics#1510
yueh added a commit to yueh/Applied-Energistics-2 that referenced this issue Jun 16, 2015
Previously it did encode the current and previous used channels into the
same as well as mask it with 0xFF. Which lead to an overflow every 256
gridnodes requiring a channel. This will not happen at > 2^31

Also removes the need to bitshift them for every access.

Fixes AppliedEnergistics#1510
@vallard192
Copy link

GridNode.java#L362
https://github.com/AppliedEnergistics/Applied-Energistics-2/blob/master/src/main/java/appeng/me/GridNode.java#L362

Is it possible this would be fixed if the conditional on Line 364 above was
changed from:

... this.getUsedChannels() > 0...
to this.getUsedChannels() >= 0...

Apologies up front because I don't have time to go check all the calls and
where this is used, but it seems like if meetsChannelRequirements is true
when a channel is used, it should also be true if no channels are used?

On Tue, Jun 16, 2015 at 6:47 AM, yueh notifications@github.com wrote:

Most likely GridNode.java#L388
https://github.com/AppliedEnergistics/Applied-Energistics-2/blob/master/src/main/java/appeng/me/GridNode.java#L388
and GridNode.java#L362
https://github.com/AppliedEnergistics/Applied-Energistics-2/blob/master/src/main/java/appeng/me/GridNode.java#L362
It will overflow to 0 every 256 channels.

We only use 2_1 byte of the int to encode the channels, so we could
change it to 2_2 bytes.
But this is only a workaround and will then fail every 65536 nodes.

I doubt it is possible to correctly implement disabled channels currently
without reimplementing alternative versions for the most important classes
like GridNode or the whole pathing.


Reply to this email directly or view it on GitHub
#1510 (comment)
.

@yueh
Copy link
Member

yueh commented Jun 16, 2015

So you want to change it in a way that every part is active even without having a channel assigned?

@vallard192
Copy link

The effect of that would be that every time you connect things to the
network, they draw power?

If so, that doesn't seem unreasonable.

But again, I'm not nearly as familiar with the code as you guys are. I
don't want it to make things work when there needs to be a channel.

On Tue, Jun 16, 2015 at 9:53 AM, yueh notifications@github.com wrote:

So you want to change it in a way that every part is active even without
having a channel assigned?


Reply to this email directly or view it on GitHub
#1510 (comment)
.

@yueh
Copy link
Member

yueh commented Jun 16, 2015

It is also already fixed in a PR.

@vallard192
Copy link

I just saw that. I think in the end that's a better fix... the problem
only comes up every 65k devices and only changes the mod for people who
play without channels.

Changing the line I suggested would change the way network power usage is
calculated and can impact everyone who uses channels as well.

Both probably aren't ideal but are decent compromises for the level of
effort required to do a more thorough rewrite.

On Tue, Jun 16, 2015 at 10:08 AM, yueh notifications@github.com wrote:

It is also already fixed in a PR.


Reply to this email directly or view it on GitHub
#1510 (comment)
.

yueh added a commit that referenced this issue Jun 16, 2015
Previously it did encode the current and previous used channels into the
same as well as mask it with 0xFF. Which lead to an overflow every 256
gridnodes requiring a channel. This will not happen at > 2^31

Also removes the need to bitshift them for every access.

Fixes #1510
yueh added a commit that referenced this issue Jun 18, 2015
Previously it did encode the current and previous used channels into the
same as well as mask it with 0xFF. Which lead to an overflow every 256
gridnodes requiring a channel. This will not happen at > 2^31

Also removes the need to bitshift them for every access.

Fixes #1510
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Self explanatory?
Projects
None yet
Development

No branches or pull requests

5 participants