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 3453
Build 4982: Bitcoin ABC Buildbot (legacy)
Build 4981: 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

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

27

Same question as above.

28

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

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

28

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/
34 lines
17 lines

Diff 5187

View Options

src/Makefile.bench.include

Loading...
View Options

src/bench/prevector_destructor.cpp

Loading...
View Options

src/prevector.h

Loading...