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

[FEATURE] Make alphabets trivial #3038

Merged
merged 2 commits into from
Oct 20, 2022
Merged

Conversation

eseiler
Copy link
Member

@eseiler eseiler commented Jun 24, 2022

std::vector optimizes assignments to std::copy (basically memcpy) iff the value_type is trivial. If not, it will use std::unintialized_copy which will use placement-new and is around 2-3 times slower.

Method Container Alphabet Iterations before Iterations after
tag::assignment_operator std::vector seqan3::aa27 705 1853
tag::assignment_operator std::vector seqan3::dna4 741 1869
tag::assignment_operator std::vector seqan3::dna5 744 1879
tag::std_copy std::vector seqan3::aa27 1691 1871
tag::std_copy std::vector seqan3::dna4 1852 1800
tag::std_copy std::vector seqan3::dna5 1781 1865
tag::uninitialized_copy std::vector seqan3::aa27 711 1789
tag::uninitialized_copy std::vector seqan3::dna4 705 1856
tag::uninitialized_copy std::vector seqan3::dna5 727 1859
tag::assignment_operator std::vector seqan::AminoAcid 740 741
tag::assignment_operator std::vector seqan::Dna 733 726
tag::assignment_operator std::vector seqan::Dna5 747 727
tag::std_copy std::vector seqan::AminoAcid 1839 1811
tag::std_copy std::vector seqan::Dna 1766 1837
tag::std_copy std::vector seqan::Dna5 1830 1847
tag::uninitialized_copy std::vector seqan::AminoAcid 734 741
tag::uninitialized_copy std::vector seqan::Dna 742 739
tag::uninitialized_copy std::vector seqan::Dna5 731 735
tag::assignment_operator seqan::String seqan::AminoAcid 1835 1938
tag::assignment_operator seqan::String seqan::Dna 1819 1910
tag::assignment_operator seqan::String seqan::Dna5 1903 1944
tag::std_copy seqan::String seqan::AminoAcid 720 721
tag::std_copy seqan::String seqan::Dna 723 734
tag::std_copy seqan::String seqan::Dna5 723 726
tag::uninitialized_copy seqan::String seqan::AminoAcid 721 719
tag::uninitialized_copy seqan::String seqan::Dna 722 723
tag::uninitialized_copy seqan::String seqan::Dna5 719 728

@vercel
Copy link

vercel bot commented Jun 24, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
seqan3 ✅ Ready (Inspect) Visit Preview Oct 18, 2022 at 11:35AM (UTC)

@seqan-actions seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Jun 24, 2022
@seqan-actions seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Jun 25, 2022
@codecov
Copy link

codecov bot commented Jun 25, 2022

Codecov Report

Base: 98.21% // Head: 98.21% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (d0011c9) compared to base (c3113fa).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3038      +/-   ##
==========================================
- Coverage   98.21%   98.21%   -0.01%     
==========================================
  Files         275      274       -1     
  Lines       12244    12204      -40     
==========================================
- Hits        12026    11986      -40     
  Misses        218      218              
Impacted Files Coverage Δ
include/seqan3/alphabet/alphabet_base.hpp 100.00% <ø> (ø)
...clude/seqan3/alphabet/aminoacid/aminoacid_base.hpp 100.00% <ø> (ø)
include/seqan3/alphabet/aminoacid/aa20.hpp 100.00% <0.00%> (ø)
include/seqan3/alphabet/aminoacid/aa27.hpp 100.00% <0.00%> (ø)
include/seqan3/alphabet/structure/wuss.hpp 100.00% <0.00%> (ø)
include/seqan3/alphabet/nucleotide/dna4.hpp 100.00% <0.00%> (ø)
include/seqan3/alphabet/nucleotide/dna5.hpp 100.00% <0.00%> (ø)
include/seqan3/alphabet/nucleotide/rna4.hpp 100.00% <0.00%> (ø)
include/seqan3/alphabet/nucleotide/rna5.hpp 100.00% <0.00%> (ø)
... and 20 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@seqan-actions seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Jun 25, 2022
@seqan-actions seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Jun 25, 2022
@seqan-actions seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Jun 25, 2022
@seqan-actions seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Oct 17, 2022
@seqan-actions seqan-actions added the lint [INTERNAL] signal for linting label Oct 17, 2022
@eseiler eseiler marked this pull request as ready for review October 17, 2022 11:17
@seqan-actions seqan-actions removed the lint [INTERNAL] signal for linting label Oct 17, 2022
@SGSSGene SGSSGene self-requested a review October 18, 2022 08:16
@eseiler eseiler changed the title [WIP] What if alphabets were trivial? [FEATURE] Make alphabets trivial Oct 18, 2022
@seqan-actions seqan-actions added the lint [INTERNAL] signal for linting label Oct 18, 2022
@eseiler
Copy link
Member Author

eseiler commented Oct 18, 2022

Blocked by #3037

@seqan-actions seqan-actions added the lint [INTERNAL] signal for linting label Oct 18, 2022
@seqan-actions seqan-actions removed the lint [INTERNAL] signal for linting label Oct 18, 2022
Copy link
Contributor

@SGSSGene SGSSGene left a comment

Choose a reason for hiding this comment

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

Improvements look great!

@eseiler eseiler merged commit f71d4de into seqan:master Oct 20, 2022
@eseiler eseiler deleted the misc/trivial_alphabet branch October 20, 2022 09:04
This pull request was closed.
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.

4 participants