Page MenuHomePhabricator
Differential D1843

Prevector Quick Destruct
ClosedPublic
Actions

Authored by deadalnix on Sep 30 2018, 14:40.

Details

Reviewers
jasonbcox
Group Reviewers
Restricted Project
Commits
rSTAGING610a16bcd049: Prevector Quick Destruct
rABC610a16bcd049: Prevector Quick Destruct
Summary
  • Add prevector destructor benchmark
  • Only call clear on prevector if it isn't trivially destructible and don't loop in clear

This is a backport of Core PR9505

Test Plan
make
./src/bench/bench_bitcoin

Diff Detail

Repository
rABC Bitcoin ABC
Branch
prevectrodestruct
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 3459
Build 4994: Bitcoin ABC Buildbot (legacy)
Build 4993: arc lint + arc unit

Event Timeline

deadalnix created this revision.Sep 30 2018, 14:40
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 30 2018, 14:40
teamcity added a subscriber: teamcity.Sep 30 2018, 14:40

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

deadalnix added a child revision: D1844: Tests: Improve benchmark precision.Sep 30 2018, 14:52
Harbormaster failed remote builds in B3450: Diff 5184!Sep 30 2018, 14:57
teamcity added a comment.Sep 30 2018, 14:57

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing failed.

deadalnix updated this revision to Diff 5187.Sep 30 2018, 15:58

Use uint8_t

teamcity added a comment.Sep 30 2018, 15:59

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Harbormaster completed remote builds in B3453: Diff 5187.Sep 30 2018, 16:16
teamcity added a comment.Sep 30 2018, 16:16

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

jasonbcox requested changes to this revision.Sep 30 2018, 16:50
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/bench/prevector_destructor.cpp
14 ↗(On Diff #5187)

Any reason t1 cannot be removed and the second resize be applied to t0?

27 ↗(On Diff #5187)

Same question as above.

28 ↗(On Diff #5187)

shouldn't this be t1?

This revision now requires changes to proceed.Sep 30 2018, 16:50
deadalnix marked 2 inline comments as done.Sep 30 2018, 17:50
deadalnix added inline comments.
src/bench/prevector_destructor.cpp
14 ↗(On Diff #5187)

You want to check the destruction of vector that are within and out of bounds with the preallocated buffer.

28 ↗(On Diff #5187)

Probably. It's definitively t0 in core.

deadalnix updated this revision to Diff 5197.Sep 30 2018, 17:53

Fix t0.clear() => t1.clear()

teamcity added a comment.Sep 30 2018, 17:53

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Harbormaster completed remote builds in B3459: Diff 5197.Sep 30 2018, 18:10
teamcity added a comment.Sep 30 2018, 18:10

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

jasonbcox accepted this revision.Sep 30 2018, 19:29
This revision is now accepted and ready to land.Sep 30 2018, 19:29
Closed by commit rABC610a16bcd049: Prevector Quick Destruct (authored by Jeremy Rubin <jeremy.l.rubin@gmail.com>, committed by deadalnix). · Explain WhySep 30 2018, 19:32
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
src/
3 lines
bench/
33 lines
17 lines

Diff 5197

View Options

src/Makefile.bench.include

Loading...
View Options

src/bench/prevector_destructor.cpp

Loading...
View Options

src/prevector.h

Loading...