Skip to content

Dpipe and minor C API fix#229

Merged
cameron314 merged 3 commits into
cameron314:masterfrom
topin89:master
Nov 12, 2020
Merged

Dpipe and minor C API fix#229
cameron314 merged 3 commits into
cameron314:masterfrom
topin89:master

Conversation

@topin89

@topin89 topin89 commented Nov 12, 2020

Copy link
Copy Markdown

First, in C API for non-win platform some macro was not defined

Second, benchmark didn't compile for two very different reasons.
First reason: some missed symbols, mostly because dlib is a compiled library with extensive checks for versioning and ODR across multiple compilation units. Luckily, that's fixed with its config and one minor bugfix.

Second, the most important: dpipe has no API for copy to queue, only exchange. And that means, you can't exchange counter_t values with int values. And you can't replace all ints with counter_t for i either, because exchange will replace it's value. So, fast way is to copy to some temp and then enqueue it. But that means double copy for every other queues, not good. And special benchmark for that is not good either. So, I changed benchmarks templates to send item_t (int in our case) and created item_t item =1 outside of loops to enqueue that.

At least now MSVC compiled it without complains.

topin89 added 3 commits November 13, 2020 01:21
Dlib usually compiles as a library. So there is some checks for
ODR and that version used is the same across compilation units.
Unfortunately, there is a bug where ODR is checked regardless of
check configs. Fixed.
DlibQueueWrapper, unlike every other wrappers, allows only
swap semantics, that is no `enqueue(T const &)`, only
`enqueue(T &)` or `enqueue(T &&)`. And that means we can't
send enqueue counter_t aka std::uint64_t to in queue. Not
sure if this is fixed as intended, but it compiles
and gives meaningful results.
@cameron314

Copy link
Copy Markdown
Owner

All good fixes, thanks!

@cameron314
cameron314 merged commit d1ac12a into cameron314:master Nov 12, 2020
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