Avoid malloc allocations in the runtime #23131

eeckstein · 2019-03-06T17:26:46Z

This PR consists of multiple commits:

Remangler: use the bump pointer allocator instead of std::vector for internal substitution data structures
Remangler: Implement the hash map for substitutions in the spirit of llvm's SmallPtrSet.
Runtime: use SmallVector instead of std::vector to avoid memory allocations in most cases.
Runtime: use lambdas to avoid allocations in std::function
Remangler: Use a bump-pointer allocated string instead of std::string

See the commit messages for details.

Fixes https://bugs.swift.org/browse/SR-10028, rdar://problem/48575729
It eliminates all allocations in the cast operation.

to-do: make the old remangler allocation free.

eeckstein · 2019-03-06T17:27:29Z

@swift-ci test

eeckstein · 2019-03-06T17:28:29Z

@compnerd Can you please review my changes in LLVMSupport.cpp? I'm not sure what's the purpose of swiftCore_EXPORTS there. I had to remove it to be able to compiler the reflection library.

swift-ci · 2019-03-06T17:50:38Z

Build failed
Swift Test Linux Platform
Git Sha - 34d373ef5f3b80d900d570c55c9abe16d8882c7a

eeckstein · 2019-03-06T18:01:08Z

@swift-ci test linux

jckarter · 2019-03-06T18:03:32Z

This is a nice performance boost, but do we know why the cast is exercising the remangler so heavily to begin with?

gottesmm · 2019-03-06T18:05:51Z

@eeckstein can we add a benchmark or an interpreter test that measures the malloc traffic to make sure we don't regress?

eeckstein · 2019-03-06T18:08:58Z

@jckarter No. That's definitely also worth investigating.

eeckstein · 2019-03-06T18:11:00Z

@gottesmm That's a good point. I'll try to add a benchmark

eeckstein · 2019-03-06T18:24:04Z

@swift-ci benchmark

swift-ci · 2019-03-06T18:24:17Z

Build failed
Swift Test Linux Platform
Git Sha - 34d373ef5f3b80d900d570c55c9abe16d8882c7a

swift-ci · 2019-03-06T18:30:47Z

Build failed
Swift Test OS X Platform
Git Sha - 34d373ef5f3b80d900d570c55c9abe16d8882c7a

compnerd · 2019-03-06T19:04:32Z

@eeckstein - the reason for the LLVMSupport doing the evil things with swiftCore_EXPORTS there is due to the fact that we don't build the swift libraries in a very logical manner. The host tools will link against LLVMSupport and use functions from it. The runtime however will use LLVMSupport and not link against it. However, even the header-only usage has a dependency on it for report_bad_alloc_error. By limiting the definitions to only when we are building swiftCore we can ensure that the other libraries do not end up with multiple definitions of the symbols (which is a hard error on Windows) and can cause other issues on other targets (due to ODR violations).

compnerd · 2019-03-06T19:05:41Z

stdlib/public/runtime/LLVMSupport.cpp

+/// on POD-like datatypes and is out of line to reduce code duplication.
+void
+#if !defined(_WIN32)
+__attribute__((__weak__, __visibility__("hidden")))


We need to use the alternatename for providing the default implementation on Windows.

Can you give me a patch for that? Or change it after this PR is merged?

Sure, I'll get you a patch (need to wait for my current build to finish first). I'd rather not break the Windows build in the meantime, because the other breakages then pile up.

eeckstein · 2019-03-06T19:14:58Z

@compnerd

By limiting the definitions to only when we are building swiftCore

OK, but LLVMSupport.cpp is only linked when building swiftCore, but not when building the host tools. So why would we need a #ifdef swiftCore_EXPORTS over the whole file?

swift-ci · 2019-03-06T19:15:08Z

Performance: -O

TEST	OLD	NEW	DELTA	RATIO
Regression
DictionaryBridgeToObjC_Access	899	994	+10.6%	0.90x (?)

Performance: -Osize

TEST	OLD	NEW	DELTA	RATIO
Regression
DictionaryBridgeToObjC_Access	879	1115	+26.8%	0.79x (?)
DataAppendDataSmallToSmall	4100	4620	+12.7%	0.89x (?)
ObjectiveCBridgeStubFromArrayOfNSString2	3470	3740	+7.8%	0.93x (?)

Performance: -Onone

TEST	OLD	NEW	DELTA	RATIO
Regression
ExclusivityIndependent	72	85	+18.1%	0.85x
UTF8Decode_InitDecoding_ascii	406	475	+17.0%	0.85x (?)
DictionaryBridgeToObjC_Access	1016	1138	+12.0%	0.89x (?)
DropWhileAnySeqCRangeIter	50510	55248	+9.4%	0.91x (?)
DropWhileAnySeqCntRange	50467	55032	+9.0%	0.92x (?)
UTF8Decode_InitDecoding	232	251	+8.2%	0.92x (?)
Improvement
DataCreateEmptyArray	581900	151250	-74.0%	3.85x
DataCreateSmallArray	629350	196150	-68.8%	3.21x
DataCreateMediumArray	65800	24080	-63.4%	2.73x
PopFrontArrayGeneric	7240	5780	-20.2%	1.25x (?)

Code size: -swiftlibs

TEST	OLD	NEW	DELTA	RATIO
Regression
libswiftRemoteMirror.dylib	364544	405504	+11.2%	0.90x
libswiftCore.dylib	3571712	3608576	+1.0%	0.99x

How to read the data

The tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.

If you see any unexpected regressions, you should consider fixing the
regressions before you merge the PR.

Noise: Sometimes the performance results (not code size!) contain false
alarms. Unexpected regressions which are marked with '(?)' are probably noise.
If you see regressions which you cannot explain you can try to run the
benchmarks again. If regressions still show up, please consult with the
performance team (@eeckstein).

Hardware Overview

  Model Name: Mac Pro
  Model Identifier: MacPro6,1
  Processor Name: 12-Core Intel Xeon E5
  Processor Speed: 2.7 GHz
  Number of Processors: 1
  Total Number of Cores: 12
  L2 Cache (per Core): 256 KB
  L3 Cache: 30 MB
  Memory: 64 GB

compnerd · 2019-03-06T19:17:32Z

The problem is that the dependency percolates through, particularly for the tests IIRC.

jckarter · 2019-03-06T19:59:07Z

There are also people who want to link Swift into binaries that also include their own copy of LLVM, and not have Swift's use of some bits of LLVMSupport for its own purpose to leak through.

shahzadlone · 2019-03-06T21:59:22Z

stdlib/public/runtime/MetadataLookup.cpp

-    std::vector<BuiltType> paramTypes;
-    std::vector<uint32_t> paramFlags;
+    SmallVector<BuiltType, 8> paramTypes;
+    SmallVector<uint32_t, 8> paramFlags;

    // Fill in the parameters.
    paramTypes.reserve(params.size());


Just checking, the reserve is still fine after?

Yes, should be fine

…internal substitution data structures This avoids mallocs in the runtime. SR-10028 rdar://problem/48575729

…llvm's SmallPtrSet. This avoids malloc calls in the common case. Only if there are more than 16 substitutions in the fixed-sized array, the overflow goes into a std::unordered_map. SR-10028 rdar://problem/48575729

…ations in most cases. This dramatically reduces the number of needed malloc calls. Unfortunately I had to add the implementation of SmallVectorBase::grow_pod to the runtime, as we don't link LLVM. This is a bad hack, but better than re-inventing a new SmallVector implementation. SR-10028 rdar://problem/48575729

Instead of capturing SubstGenericParametersFromMetadata and SubstGenericParametersFromWrittenArgs by value, capture by reference. This avoids those instances to be copied and thus avoids a lot of mallocs. SR-10028 rdar://problem/48575729

Done by replacing DemanglerPrinter with a bump-pointer allocated CharVector buffer. This avoids malloc calls. SR-10028 rdar://problem/48575729

…meters and arguments don't match. This can happen if _typeByName() is called with an invalid mangled type name.

eeckstein · 2019-03-06T22:43:06Z

@compnerd I re-added #if defined(swiftCore_EXPORTS) in LLVMSupport.cpp.
About alternatename for Windows: I don't know how to do that and I cannot test it. Can you please do that change or give me a patch?

eeckstein · 2019-03-06T22:43:18Z

@swift-ci test

eeckstein · 2019-03-06T22:43:43Z

@swift-ci test

compnerd · 2019-03-06T23:32:16Z

Yeap, I'll create a modification of this patch for that.

swift-ci · 2019-03-07T00:45:04Z

Build failed
Swift Test OS X Platform
Git Sha - 0dd2495

eeckstein · 2019-03-07T00:50:37Z

@swift-ci test macOS

eeckstein · 2019-03-07T00:50:58Z

@swift-ci test macOS

compnerd · 2019-03-07T17:26:47Z

@eeckstein - btw, for future reference (and mostly myself) - the other use of LLVMSupport.cpp is in swiftReflection.

tested locally

compnerd · 2019-03-07T17:36:43Z

@eeckstein - seems that without the aliasing it also works. I imagine that the definition here is just being given precedence to the one in LLVMSupport. Seems to be working in my local test.

eeckstein · 2019-03-07T18:13:28Z

@compnerd Great, thanks for testing it!

Reintroduce some changes done in swiftlang#23131 to hide some weak symbols generated when compiling as part of libswiftDemangle.dylib, which may introduce a performance regression. Addresses rdar://63454568

eeckstein requested review from DougGregor and compnerd March 6, 2019 17:26

compnerd previously requested changes Mar 6, 2019

View reviewed changes

eeckstein force-pushed the runtime-malloc-removal branch from 34d373e to 5666bf2 Compare March 6, 2019 21:26

shahzadlone reviewed Mar 6, 2019

View reviewed changes

eeckstein added 6 commits March 6, 2019 14:37

Remangler: use the bump pointer allocator instead of std::vector for …

de51f8c

…internal substitution data structures This avoids mallocs in the runtime. SR-10028 rdar://problem/48575729

Remangler: Use a bump-pointer allocated string instead of std::string

d7fd45d

Done by replacing DemanglerPrinter with a bump-pointer allocated CharVector buffer. This avoids malloc calls. SR-10028 rdar://problem/48575729

runtime: bail in createBoundGenericType if the number of generic para…

0dd2495

…meters and arguments don't match. This can happen if _typeByName() is called with an invalid mangled type name.

eeckstein force-pushed the runtime-malloc-removal branch from 5666bf2 to 0dd2495 Compare March 6, 2019 22:41

eeckstein merged commit ae189ef into swiftlang:master Mar 7, 2019

eeckstein deleted the runtime-malloc-removal branch March 7, 2019 18:13

eeckstein mentioned this pull request Mar 12, 2019

Avoid malloc allocations in the runtime, part 2 #23249

Merged

edymtt mentioned this pull request May 21, 2020

[build] hide some symbols in LLVMSupport #31951

Closed

eeckstein mentioned this pull request Mar 7, 2019

[SR-10028] one single 'handler as? _ChannelOutboundHandler' allocates 144 times, every time it's executed #52431

Closed

Avoid malloc allocations in the runtime #23131

Avoid malloc allocations in the runtime #23131

Uh oh!

Conversation

eeckstein commented Mar 6, 2019

Uh oh!

eeckstein commented Mar 6, 2019

Uh oh!

eeckstein commented Mar 6, 2019

Uh oh!

swift-ci commented Mar 6, 2019

Uh oh!

eeckstein commented Mar 6, 2019

Uh oh!

jckarter commented Mar 6, 2019

Uh oh!

gottesmm commented Mar 6, 2019

Uh oh!

eeckstein commented Mar 6, 2019

Uh oh!

eeckstein commented Mar 6, 2019

Uh oh!

eeckstein commented Mar 6, 2019

Uh oh!

swift-ci commented Mar 6, 2019

Uh oh!

swift-ci commented Mar 6, 2019

Uh oh!

compnerd commented Mar 6, 2019

Uh oh!

compnerd Mar 6, 2019

Choose a reason for hiding this comment

Uh oh!

eeckstein Mar 6, 2019

Choose a reason for hiding this comment

Uh oh!

compnerd Mar 6, 2019

Choose a reason for hiding this comment

Uh oh!

eeckstein commented Mar 6, 2019

Uh oh!

swift-ci commented Mar 6, 2019

Performance: -O

Performance: -Osize

Performance: -Onone

Code size: -swiftlibs

Uh oh!

compnerd commented Mar 6, 2019

Uh oh!

jckarter commented Mar 6, 2019

Uh oh!

shahzadlone Mar 6, 2019

Choose a reason for hiding this comment

Uh oh!

eeckstein Mar 6, 2019

Choose a reason for hiding this comment

Uh oh!

eeckstein commented Mar 6, 2019

Uh oh!

eeckstein commented Mar 6, 2019

Uh oh!

eeckstein commented Mar 6, 2019

Uh oh!

compnerd commented Mar 6, 2019

Uh oh!

swift-ci commented Mar 7, 2019

Uh oh!

eeckstein commented Mar 7, 2019

Uh oh!

eeckstein commented Mar 7, 2019

Uh oh!

compnerd commented Mar 7, 2019

Uh oh!

compnerd commented Mar 7, 2019

Uh oh!

eeckstein commented Mar 7, 2019

Uh oh!

Uh oh!