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

Performance improvements for the energygrid #3051

Merged
merged 7 commits into from Aug 24, 2017

Conversation

Projects
None yet
3 participants
@yueh
Member

yueh commented Aug 19, 2017

Reworked the old recursive approach to a queue based loop.
Extract will try to prefer the next grid with the hightest amount of stored energy.
Inject will try to prefer the grid with the lowest percentage stored.
Other operations are first come, first serve.


Edit

This replaces the old not really working buffer with a special
IAEPowerStorage acting as buffer/proxy for the local energy demand as
well as temporary overflow should something provide more energy than
requested.

Currently it set to hold a maximum of 1000 AE (+ optional overflow until
consumed). It will only be used locally, no other grid can use it to
avoid starving the neighbor grids before finding a energy cell.

1000 AE is certainl excessive for a local storage in nearly all cases. Around 10 to 100 would certainly be more fitting. But a local network with 8 drives full of 64k cells will extract about 180 AE per tick, which should be about the amount a local buffer should provide to avoid reaching out to energy cells in other grids. Thus 200 AE might be fine.

Another idea might be to link it to the idle drain. So every gridnode would add/remove their idle draw to the maximum on top off a small buffer to accomendate for the energy use by channels.

Fixes #1004

Performance improvements for the energygrid
Reworked the old recursive approach to a queue based loop.
Extract will try to prefer the next grid with the hightest amount of
stored energy.
Inject will try to prefer the grid with the lowest percentage stored.
Other operations are first come, first serve.

Fixes #1004
if( !seen.contains( eg ) )
{
demand += eg.getProviderEnergyDemand( amt - demand, seen );
}
}
catch( final GridAccessException e )

This comment has been minimized.

@orod-org

orod-org Aug 19, 2017

MAJOR Either log or rethrow this exception. rule

@orod-org

orod-org Aug 19, 2017

MAJOR Either log or rethrow this exception. rule

final IEnergyGrid eg = this.outerProxy.getEnergy();
stuff.add( eg );
}
catch( final GridAccessException e )

This comment has been minimized.

@orod-org

orod-org Aug 19, 2017

MAJOR Either log or rethrow this exception. rule

@orod-org

orod-org Aug 19, 2017

MAJOR Either log or rethrow this exception. rule

yueh added some commits Aug 20, 2017

Added a local buffer storage to EnergyGrid
This replaces the old not really working buffer with a special
IAEPowerStorage acting as buffer/proxy for the local energy demand as
well as temporary overflow should something provide more energy than
requested.

Currently it set to hold a maximum of 1000 AE (+ optional overflow until
consumed). It will only be used locally, no other grid can use it to
avoid starving the neighbor grids before finding a energy cell.

@yueh yueh added this to the rv5.alpha - 1.12 milestone Aug 20, 2017

Fixes IExternalPowerSink
All implementations currently depend on the network demand being a valid
source, which might not be true.

Further it can cause the sink to iterate the network twice (demand and
inject) and both again for simulate and modulate.

Also it did not return the actual leftover amount instead of relying on
the demand matching it.
}
return amt - insertable;
}
protected void PowerEvent( final PowerEventType x )

This comment has been minimized.

@orod-org

orod-org Aug 20, 2017

MINOR Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule

@orod-org

orod-org Aug 20, 2017

MINOR Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule

}
return 0;
final IEnergyGrid grid = this.getProxy().getEnergy();
final double leftOver = grid.injectPower( power, mode );

This comment has been minimized.

@orod-org

orod-org Aug 20, 2017

MINOR Immediately return this expression instead of assigning it to the temporary variable "leftOver". rule

@orod-org

orod-org Aug 20, 2017

MINOR Immediately return this expression instead of assigning it to the temporary variable "leftOver". rule

@@ -84,11 +85,8 @@ protected double funnelPowerIntoStorage( final double power, final Actionable mo
{
final IEnergyGrid grid = this.getProxy().getEnergy();
final double leftOver = grid.injectPower( power, mode );

This comment has been minimized.

@orod-org

orod-org Aug 20, 2017

MINOR Immediately return this expression instead of assigning it to the temporary variable "leftOver". rule

@orod-org

orod-org Aug 20, 2017

MINOR Immediately return this expression instead of assigning it to the temporary variable "leftOver". rule

Minor fixes related to removing nodes from a grid.
The grid did remove IStackWatcherHost not IEnergyWatcherHost, this was
fine for AE2 as only level emitters use it and they implement both.
But not for potential addons.

Also they would potentually not being removed as the are indexed by the
gridnodes not the machine.
@orod-org

This comment has been minimized.

Show comment
Hide comment
@orod-org

orod-org Aug 21, 2017

SonarQube analysis reported 17 issues

  • MAJOR 6 major
  • MINOR 11 minor

Watch the comments in this conversation to review them.

Top 10 extra issues

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. MAJOR AEBasePoweredTile.java#L102: Remove this unused method parameter "maxRequired". rule
  2. MAJOR AEBasePoweredTile.java#L272: Remove this conditional structure or edit its code blocks so that they're not all the same. rule
  3. MAJOR AEBasePoweredTile.java#L281: Merge this if statement with the enclosing one. rule
  4. MAJOR AEBasePoweredTile.java#L303: Merge this if statement with the enclosing one. rule
  5. MINOR IC2PowerSinkAdapter.java#L66: Remove this method to simply inherit it. rule
  6. MINOR EnergyGridCache.java#L130: Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule
  7. MINOR EnergyGridCache.java#L144: Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule
  8. MINOR TileCharger.java#L85: Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule
  9. MINOR TileCharger.java#L101: Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule
  10. MINOR AEBasePoweredTile.java#L85: Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule

orod-org commented Aug 21, 2017

SonarQube analysis reported 17 issues

  • MAJOR 6 major
  • MINOR 11 minor

Watch the comments in this conversation to review them.

Top 10 extra issues

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. MAJOR AEBasePoweredTile.java#L102: Remove this unused method parameter "maxRequired". rule
  2. MAJOR AEBasePoweredTile.java#L272: Remove this conditional structure or edit its code blocks so that they're not all the same. rule
  3. MAJOR AEBasePoweredTile.java#L281: Merge this if statement with the enclosing one. rule
  4. MAJOR AEBasePoweredTile.java#L303: Merge this if statement with the enclosing one. rule
  5. MINOR IC2PowerSinkAdapter.java#L66: Remove this method to simply inherit it. rule
  6. MINOR EnergyGridCache.java#L130: Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule
  7. MINOR EnergyGridCache.java#L144: Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule
  8. MINOR TileCharger.java#L85: Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule
  9. MINOR TileCharger.java#L101: Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule
  10. MINOR AEBasePoweredTile.java#L85: Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule

@yueh yueh merged commit 1513ba3 into rv5-1.12 Aug 24, 2017

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
jenkins Success
Details
sonarqube SonarQube reported 17 issues, no criticals or blockers

@yueh yueh deleted the feature-energygrid-improvements branch Oct 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment