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

[SR-3340] Evaluate the ABI for existential container storage. #45928

Closed
atrick opened this issue Dec 5, 2016 · 5 comments
Closed

[SR-3340] Evaluate the ABI for existential container storage. #45928

atrick opened this issue Dec 5, 2016 · 5 comments
Assignees
Labels
affects ABI Flag: Affects ABI compiler The Swift compiler itself task

Comments

@atrick
Copy link
Contributor

atrick commented Dec 5, 2016

Previous ID SR-3340
Radar rdar://problem/18070569
Original Reporter @atrick
Type Task
Status Resolved
Resolution Won't Do
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Task, AffectsABI
Assignee @aschwaighofer
Priority Medium

md5: 31d69b1f204991e8e7149c604841e45d

Issue Description:

We currently reserve 3 words of inline storage for existential containers. We may want to reevaluate this decision for codifying it. This might be too small for important patterns. e.g. NSRect does not fit.

We also need to consider whether we want to stack allocate local existential boxes (when the type doesn't fit in the inline buffer).

This is a secondary ABI concern. i.e. it would be nice to reevaluate the current design before the final ABI lock down.

@atrick
Copy link
Contributor Author

atrick commented Jul 1, 2017

If we can assume that String will eventually have a single word representation, then 3 words is likely too large.

IMO we should consider the tradeoff between a single-word existential vs. a two-word existential. How often do we represent pairs?

@atrick
Copy link
Contributor Author

atrick commented Jul 1, 2017

Please file a separate bug for this issue if it makes sense...

From talking to Arnold, it seems we should consider limiting existential container storage to a single word for protocol witness tables. Why can't inherited conformance result in chained witness tables?

I think the an unbounded protocol container size is a bigger performance cliff than a level of indirection on the witness tables.

@aschwaighofer
Copy link
Contributor

It is implied in this bug that we have a sensible way of evaluating the decision what size the inline storage should be based on some benchmarks.

I claim we don't have the set of benchmarks available to make a sensible data based decision:

I think for a truly meaning full data base decision we would have to evaluate a representative set of programs against a metric like minimizing heap memory waste vs performance over the dynamic execution of this set of programs. We would need system'y benchmarks (i.e larger applications that model properties of large frameworks or system applications).

I posit that we don't have this set of benchmarks available (our swift/benchmarks certainly don't qualify IMO) nor do we have the time to construct them.

Short of that I think that it is going to be intuition what a good size is based on some sensible rules:

  • One that we expect to miminize memory waste and maximizes performance.

  • This is going to be around the size where we are today (IMO).

  • The size should work well with common standard library types:

  • String is going to be two words

  • StringSlice is going to be three words

This favors the status quo. The status quo is further favored by the fact that existing applications rely on it implicitly.

What would we get out of an experiment running the benchmarks:

  • Assume 2 word buffer regresses unacceptly; then we would argue we have to stay with a 3 word buffer (which is what we are proposing without the experiment).

  • Assume it does not; what would be the conclusion. I would not want to move to a 2 word buffer based on this because i don’t believe that the benchmark to be representative of performance or memory requirements in a system setting.

@aschwaighofer
Copy link
Contributor

Explicitly stating the size trade-off:

If we make the buffer smaller and types are bigger than the new inline buffer size that would have fit the original inline buffer we pay the size overhead of a heap object allocation (heap object header, memory locality) ... and of course the performance costs of allocation + reference counting.

If we make the buffer bigger, for types smaller than the allocated inline buffer memory is wasted on inline buffer size that is unused.

@bob-wilson
Copy link
Contributor

We've had a number of discussions about this. On the forums:

https://forums.swift.org/t/sr-3340-evaluate-the-abi-for-existential-container-storage-size-of-the-inline-value-buffer/8965

https://forums.swift.org/t/state-of-string-abi-performance-ergonomics-and-you/7397/21

Several people expressed the opinion that for some future version of Swift, it would be nice to have the ability to increase the buffer size beyond the ABI requirement for particular apps or even particular types, but there was no clear consensus on whether the default inline buffer should be 2 or 3 words.

Some of us got together in person to discuss it last week and decided to stick with the status quo and leave the buffer size at 3 words. The main reasons were:

  • We don't have a good way to quantitatively evaluate the tradeoff between 2 and 3 words.

  • We expect the difference to be small, and there are more important things that we should work on before declaring ABI stability.

  • Existing apps have been tuned to work well with the current 3-word buffers, and changing that could be disruptive in some cases.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects ABI Flag: Affects ABI compiler The Swift compiler itself task
Projects
None yet
Development

No branches or pull requests

3 participants