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.

Sign up for GitHub

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

Jump to bottom

<array>: <array> includes too much stuff #462

Closed
BillyONeal opened this issue Jan 27, 2020 · 3 comments · Fixed by #482
Closed

<array>: <array> includes too much stuff #462

BillyONeal opened this issue Jan 27, 2020 · 3 comments · Fixed by #482
Labels
fixed Something works now, yay! throughput Must compile faster

Comments

@BillyONeal
Copy link
Member

BillyONeal commented Jan 27, 2020 •
edited
Loading

In @ned14 's recent STL header benchmark results, <array> stands out as the most expensive STL container header. See https://www.reddit.com/r/cpp/comments/eumou7/stl_header_token_parsing_benchmarks_for_vs2017/ . This is because <array> includes <iterator> (because reverse_iterator used to be in there?), <tuple> (because when we authored <array> the tuple interface wasn't in <type_traits> yet?), and <algorithm> (for _Swap_ranges_unchecked). These are needless overhead for the functionality <array> offers.

It probably would make sense to promote _Swap_ranges_unchecked to <xutility> or similar with all the other "optimized for trivial things" algorithms like copy,
@BillyONeal BillyONeal added enhancement Something can be improved throughput Must compile faster decision needed We need to choose something before working on this labels Jan 27, 2020
@BillyONeal
Copy link
Member Author

BillyONeal commented Jan 27, 2020 •
edited
Loading

It might be nice to actually go further and make <array> includable without dragging in all of our allocator support machinery <xutility> and <xmemory> currently drag in, but that's a more substantial refactoring effort to break those two headers apart.

@StephanTLavavej
Copy link
Member

I'd like to see <iterator>, <tuple>, and <algorithm> avoided in one change, before considering whether we should disentangle <xutility> and <xmemory>. I'm not really concerned about fractionating the <array> breaking changes into two smaller doses; instead I am concerned about disrupting every other STL header.

@BillyONeal
Copy link
Member Author

That sounds fair to me

@BillyONeal BillyONeal removed the decision needed We need to choose something before working on this label Jan 27, 2020
BillyONeal added a commit to BillyONeal/STL that referenced this issue Feb 5, 2020
@BillyONeal
Reduce the amount of content included by <array>.
6bc5030 
Resolves microsoftGH-462.

* Demote `back_inserter` and `iterator` to `<iterator>`.
* Demote `_Yarn` to `<xlocinfo>`.
* Demote `_Tidy_guard`, `_Tidy_deallocate_guard`, and `_Nothrow_compare` to `<xmemory>`.
* Promote `_Swap_ranges_unchecked` to `<xutility>`.
* Change `<array>` to include only `<xutility>`.
@BillyONeal BillyONeal mentioned this issue Feb 5, 2020
Merged
4 tasks
@StephanTLavavej StephanTLavavej removed the enhancement Something can be improved label Feb 5, 2020
BillyONeal added a commit that referenced this issue Feb 21, 2020
@BillyONeal
Reduce the amount of content included by <array> (#482)
482f1d8 
* Reduce the amount of content included by <array>.

Resolves GH-462.

* Demote `back_inserter` and `iterator` to `<iterator>`.
* Demote `_Yarn` to `<xlocinfo>`.
* Demote `_Tidy_guard`, `_Tidy_deallocate_guard`, and `_Nothrow_compare` to `<xmemory>`.
* Promote `_Swap_ranges_unchecked` to `<xutility>`.
* Change `<array>` to include only `<xutility>`.

* Un-demote iterator.

* Workaround many RWC projects that expected std::min and std::max to come from <array>.

* Remove the `_STL_ASSERT` from `std::min` and `std::max`. We normally guard every `op<` with debug checks, but in this case we aren't using it to enforce something like a container invariant; the number of bad op<s we catch with it are likely microscopic.
* Delete `_Min_value` and `_Max_value` from `<utility>`.
* Move `min` and `max` to `<utility>` (in the exact position as the old `_Min_value` and `_Max_value`)
* Change all existing callers of `_Min_value` and `_Max_value` to call `(_STD min)` and `(_STD max)`, respectively.

* Homogenize vector algorithm guards.
@BillyONeal BillyONeal added the fixed Something works now, yay! label Feb 21, 2020
@BillyONeal BillyONeal mentioned this issue May 26, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
fixed Something works now, yay! throughput Must compile faster
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Reduce the amount of content included by <array> BillyONeal/STL
2 participants
@BillyONeal @StephanTLavavej