R4R: Simulation Queued Time Operations #2348
Conversation
sunnya97
requested review from
cwgoes,
ebuchman and
rigelrozanski
as code owners
|
Why is this a separate operation? Why not just add a BlockTime field to FutureOperation, and then just searching by either? (Or alternatively have NewFutureOperation methods) |
sunnya97
force-pushed
the
sunny/simulation_time
branch
from
dfdb4f9 to
654cc2d
Compare
|
@ValarDragon Good point. |
Codecov Report
@@ Coverage Diff @@
## develop #2348 +/- ##
========================================
Coverage 64.97% 64.97%
========================================
Files 135 135
Lines 8418 8418
========================================
Hits 5470 5470
Misses 2587 2587
Partials 361 361 |
sunnya97
force-pushed
the
sunny/simulation_time
branch
from
d93c4f5 to
058e470
Compare
| } else { | ||
| queuedOperations[futureOp.BlockHeight] = []Operation{futureOp.Op} | ||
| index := sort.Search(len(queuedTimeOperations), func(i int) bool { return queuedTimeOperations[i].BlockTime.After(futureOp.BlockTime) }) |
There was a problem hiding this comment.
Since we have to do a search, can you make a TODO comment to make the queued time ops a sorted data structure? (This can then be addressed #postlaunch or if it bottlenecks the simulation)
Right now we have to copy all the time ops on insertion / deletion, which is an O(N) operation.
| @@ -274,6 +284,27 @@ func runQueuedOperations(queueOperations map[int][]Operation, height int, tb tes | |||
| return 0 | |||
| } | |||
|
|
|||
| // nolint: errcheck | |||
There was a problem hiding this comment.
We do want this passing errcheck though. Is there a specific line/error your trying to skip?
| @@ -140,8 +141,10 @@ func SimulateFromSeed(tb testing.TB, app *baseapp.BaseApp, | |||
| // Run queued operations. Ignores blocksize if blocksize is too small | |||
| numQueuedOpsRan := runQueuedOperations(operationQueue, int(header.Height), tb, r, app, ctx, keys, logWriter, displayLogs, event) | |||
| thisBlockSize -= numQueuedOpsRan | |||
| numQueuedTimeOpsRan := runQueuedTimeOperations(timeOperationQueue, header.Time, tb, r, app, ctx, keys, logWriter, displayLogs, event) | |||
| thisBlockSize -= numQueuedTimeOpsRan | |||
There was a problem hiding this comment.
Can you combine the lines subtracting the block size?
alexanderbez
changed the title
sunnya97
changed the title
|
Simulator fail isn't from this PR. |
closes #2349
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
Updated relevant documentation (
docs/)Added entries in
PENDING.mdwith issue #rereviewed
Files changedin the github PR explorerFor Admin Use: