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

Restore performance of replay #312

Merged
merged 4 commits into from
Aug 31, 2019
Merged

Restore performance of replay #312

merged 4 commits into from
Aug 31, 2019

Conversation

smarr
Copy link
Owner

@smarr smarr commented Aug 13, 2019

This PR restores the performance to the same level as before #308 was merged.

The main problem introduced was that parts of the trace file were read multiple times.
This caused the replay test jobs to take 2-3 times more time on non-SSD systems.

@smarr smarr added bug Fixes an issue, incorrect implementation enhancement Improves the implementation with something noteworthy labels Aug 13, 2019
@smarr smarr added this to the v0.8.0 milestone Aug 13, 2019
@smarr smarr self-assigned this Aug 13, 2019
@smarr smarr force-pushed the da/replay-opts branch 3 times, most recently from 198825f to 38c73b1 Compare August 14, 2019 07:06
daumayr and others added 4 commits August 17, 2019 19:10
- focus methods on main purpose
- use a static trace file to avoid opening trace multiple times
- fold TraceRecord ids into enum and move to own file
- handle statistics in EventParseContext, too
- record subtrace length during scanning to avoid over-reading
- make TraceParser an object and instantiate in VM
  - this change requires some adaptation of primitives
  - for instance, TicksPrim needs to be explicitly a
    BasicPrimitiveOperation now
- make buffer size testable
- move initial trace scan to initialization that’s done explicitly
- don’t swallow exceptions
- store offset and length for context subtrace
- if Subtrace length is not set, also use standardBufferSize
  this should only be relevant for the last subtrace

Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
@smarr smarr marked this pull request as ready for review August 27, 2019 12:50
Copy link
Contributor

@daumayr daumayr left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the parser performance, changes look good

@smarr
Copy link
Owner Author

smarr commented Aug 29, 2019

@daumayr thanks! Did you see the TraceParserTests?

They are fairly minimal, but I hope this gives us a bit of confidence when stuff gets changed around.

Will merge later today or tomorrow, I hope.

@daumayr
Copy link
Contributor

daumayr commented Aug 29, 2019

yes, is saw the tests

@smarr smarr merged commit 6e77b0a into dev Aug 31, 2019
@smarr smarr deleted the da/replay-opts branch August 31, 2019 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes an issue, incorrect implementation enhancement Improves the implementation with something noteworthy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants