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

support BOOST_URL_DISABLE_THREADS #689

Merged

Conversation

alandefreitas
Copy link
Member

fix #684

@alandefreitas alandefreitas marked this pull request as ready for review February 20, 2023 23:26
@vinniefalco
Copy link
Member

Where do we detect single-threaded toolchains? The user has to set this manually?

@alandefreitas
Copy link
Member Author

@ropieur could you try this branch? This implementation uses BOOST_SYSTEM_DISABLE_THREADS as a reference.

@cppalliance-bot
Copy link

@codecov
Copy link

codecov bot commented Feb 20, 2023

Codecov Report

Merging #689 (58ed304) into develop (b49fd36) will decrease coverage by 0.02%.
The diff coverage is 94.44%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #689      +/-   ##
===========================================
- Coverage    97.13%   97.12%   -0.02%     
===========================================
  Files          152      152              
  Lines         8441     8445       +4     
===========================================
+ Hits          8199     8202       +3     
- Misses         242      243       +1     
Impacted Files Coverage Δ
include/boost/url/grammar/detail/recycled.hpp 100.00% <ø> (ø)
include/boost/url/grammar/recycled.hpp 100.00% <ø> (ø)
include/boost/url/grammar/detail/impl/recycled.ipp 95.65% <92.85%> (-4.35%) ⬇️
include/boost/url/grammar/impl/recycled.hpp 87.93% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b49fd36...58ed304. Read the comment docs.

@alandefreitas
Copy link
Member Author

std::lock_guard<
std::mutex> m(all_reports_.m);
#endif
Copy link
Member

Choose a reason for hiding this comment

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

You can just make count and bytes atomic instead of using a mutex.

@@ -90,8 +90,14 @@ class recycled
{
T t;
U* next = nullptr;

#if !defined(BOOST_URL_DISABLE_THREADS)
Copy link
Member

@pdimov pdimov Feb 21, 2023

Choose a reason for hiding this comment

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

This is unnecessary. Single threaded C++11 platforms have a working <atomic>.

Copy link
Member Author

Choose a reason for hiding this comment

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

<atomic>

@cppalliance-bot
Copy link

@cppalliance-bot
Copy link

@alandefreitas alandefreitas merged commit 1ff5ea7 into boostorg:develop Feb 22, 2023
old_count_max, new_count))
{}

std::size_t new_bytes = a.bytes.fetch_add(n);
Copy link
Member

Choose a reason for hiding this comment

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

fetch_add returns the old value, not the new one. a.bytes += n

Copy link
Member Author

Choose a reason for hiding this comment

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

🤦‍♂️

Copy link
Member Author

Choose a reason for hiding this comment

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

This?

#691

@ropieur
Copy link

ropieur commented Feb 23, 2023

@ropieur could you try this branch? This implementation uses BOOST_SYSTEM_DISABLE_THREADS as a reference.

I will have to check internally with my colleagues how we can proceed next week when I am back at work, because we are building boost as a 3rdparty all at once. This will require from us to merge your changes with boost 1.81.0 and tweak our building procedure. I'll keep you posted.

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.

boost url does not build in single thread
5 participants