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

simulation: Switch the log method from a single string to string buil… #2282

Merged
merged 3 commits into from
Sep 9, 2018

Conversation

ValarDragon
Copy link
Contributor

@ValarDragon ValarDragon commented Sep 9, 2018

…ders

This rapidly speeds up simulation in testing mode.
ref #1924

The idea here is basically instead of using a single "log" which everyone appends to all the time, instead have a log builder, which we have a centralized way of writing to/fro. Additionally invariants no longer print out the log, but instead return an error. This enables them to be easily reused / be located elsewhere from the simulation module.

In general it makes the types simulation wants you to make more extendable and easier to understand.

  • Targeted PR against correct branch (see CONTRIBUTING.md)

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

  • Wrote tests

  • Added entries in PENDING.md with issue #

  • rereviewed Files changed in the github PR explorer


For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@@ -157,7 +157,7 @@ test_sim_gaia_nondeterminism:

test_sim_gaia_fast:
@echo "Running quick Gaia simulation. This may take several minutes..."
@go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=150 -v -timeout 24h
@go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=200 -v -timeout 24h
Copy link
Contributor Author

@ValarDragon ValarDragon Sep 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After 200 it slows down again cuz of the governance slashing issue. But it burns through the first 200 in ~20 seconds!

…ders

This rapidly speeds up simulation in testing mode.
@codecov
Copy link

codecov bot commented Sep 9, 2018

Codecov Report

Merging #2282 into develop will increase coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #2282      +/-   ##
===========================================
+ Coverage    63.82%   63.83%   +0.01%     
===========================================
  Files          140      140              
  Lines         8668     8666       -2     
===========================================
  Hits          5532     5532              
+ Misses        2756     2754       -2     
  Partials       380      380

@ValarDragon
Copy link
Contributor Author

Also once this PR is merged, it will be super easy to make simulation print out the log on a panic

@cwgoes
Copy link
Contributor

cwgoes commented Sep 9, 2018

Huh is this faster than just using a byteslice instead of a string for the log messages? Seems like a possible memory leak to keep so many stringbuilders around.

@ValarDragon
Copy link
Contributor Author

Using a single byte slice still has the re-allocation problem. (E.g when it's too full you need to re allocate the whole thing). My thinking with this is that it will be easier to then just start to purge these from memory by writing to disk every few blocks, and then deleting that string builder.

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

PositivePowerInvariant(k)(t, app, log)
ValidatorSetInvariant(k)(t, app, log)
return func(app *baseapp.BaseApp) error {
err := SupplyInvariants(ck, k, am)(app)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would that Golang had monads

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can only wish :(

@cwgoes cwgoes merged commit 173ed6a into develop Sep 9, 2018
@cwgoes
Copy link
Contributor

cwgoes commented Sep 9, 2018

Using a single byte slice still has the re-allocation problem. (E.g when it's too full you need to re allocate the whole thing). My thinking with this is that it will be easier to then just start to purge these from memory by writing to disk every few blocks, and then deleting that string builder.

OK!

@cwgoes cwgoes deleted the dev/simulation_logging branch September 9, 2018 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants