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

ThreadLocal<T> not using the JIT fast path for TLS access #18370

Closed
fanyang-mono opened this issue Jan 7, 2020 · 2 comments · Fixed by #18377
Closed

ThreadLocal<T> not using the JIT fast path for TLS access #18370

fanyang-mono opened this issue Jan 7, 2020 · 2 comments · Fixed by #18377
Labels
area-Runtime: Performance To track performance improvement related issues task

Comments

@fanyang-mono
Copy link
Contributor

fanyang-mono commented Jan 7, 2020 •
edited

Steps to Reproduce

Make the following change to mono/netcore/build.sh
Screen Shot 2020-01-07 at 11 20 17 AM

Revert the following change:
cfba7c5#diff-274660eb4b1b1d963a330b471e10f41c

Then run the following script by following the instructions in it.
profileMonoScript.sh.zip

Current Behavior

System_Threading_ThreadLocal_1_T_REF_get_Value/mono_class_static_field_address is one of the hotspots according to the following flamegraph
Screen Shot 2020-01-07 at 10 29 24 AM

Expected Behavior

System_Threading_ThreadLocal_1_T_REF_get_Value/mono_class_static_field_address should take a small portion of time of the whole program.

On which platforms did you notice this

[ ] macOS
[x] Linux
[ ] Windows

Version Used:

mono master: 61eb2be from Dec 30th, 2019
@fanyang-mono fanyang-mono added task area-Runtime: Performance To track performance improvement related issues labels Jan 7, 2020
@fanyang-mono
Copy link
Contributor Author

Quote from @lambdageek:
"It looks like there is contention for the domain lock which protects the hash table that contains the offsets of the special static fields.
We could investigate using a concurrent hash table – ie replace GHashTable by MonoConcGHashTable which is lock free and would perform better for concurrent lookups."

@vargaz
Copy link
Contributor

vargaz commented Jan 7, 2020

The JIT is supposed to emit optimized code for TLS access, mono_class_static_field_address () is the slow path. It looks like the fast path is not used in some cases like generic shared methods which is what ThreadLocal is.

vargaz added a commit to vargaz/mono that referenced this issue Jan 8, 2020
@vargaz
[jit] Implement fast TLS access in gshared methods.
bab47ca 
Fixes mono#18370.
@vargaz vargaz mentioned this issue Jan 8, 2020
Merged
@lambdageek lambdageek changed the title [ThreadLocal] Contention for the domain lock ThreadLocal<T> not using the JIT fast path for TLS vars Jan 8, 2020
@lambdageek lambdageek changed the title ThreadLocal<T> not using the JIT fast path for TLS vars ThreadLocal<T> not using the JIT fast path for TLS access Jan 8, 2020
vargaz added a commit that referenced this issue Jan 8, 2020
@vargaz
[jit] Implement fast TLS access in gshared methods. (#18377)
e999846 
Fixes #18370.
ManickaP pushed a commit to ManickaP/runtime that referenced this issue Jan 20, 2020
@vargaz
[jit] Implement fast TLS access in gshared methods. (mono/mono#18377)
99db20f 
Fixes mono/mono#18370.

Commit migrated from mono/mono@e999846
jonpryor pushed a commit to xamarin/xamarin-android that referenced this issue Mar 13, 2020
@directhex
Bump to mono/mono@d90665a4 (#4219)
8599d63 
Changes: mono/api-snapshot@53a841f...5b8247e

	$ git diff --shortstat 53a841ff...5b8247e2
	 9 files changed, 572 insertions(+), 13 deletions(-)

Changes: mono/corefx@1cdb9c2...7c9e215

	$ git diff --shortstat 1cdb9c20...7c9e2158
	 24 files changed, 2393 insertions(+), 396 deletions(-)

Changes: dotnet/cecil@a6a7f5c...8021f3f

	$ git diff --shortstat a6a7f5c0...8021f3fb
	 16 files changed, 98 insertions(+), 25 deletions(-)

Changes: dotnet/linker@e8d054b...e1c7a72

	$ git diff --shortstat e8d054bf...e1c7a729
	 220 files changed, 9758 insertions(+), 3165 deletions(-)

Changes: mono/mono@2ff8988...d90665a

	$ git diff --shortstat 2ff89885...d90665a4
	 612 files changed, 20193 insertions(+), 10239 deletions(-)

Context: mono/mono#9726
Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1048838
Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1050615
Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1069059
Context: mono/mono#10643
Context: mono/mono#10651
Context: mono/mono#12022
Context: mono/mono#12995
Context: mono/mono#15612
Context: mono/mono#16513
Context: mono/mono#16588
Context: mono/mono#16778
Context: mono/mono#16969
Context: mono/mono#17140
Context: mono/mono#17601
Context: mono/mono#17869
Context: mono/mono#17878
Context: mono/mono#17916
Context: mono/mono#17926
Context: mono/mono#17980
Context: mono/mono#18006
Context: mono/mono#18019
Context: mono/mono#18020
Context: mono/mono#18030
Context: mono/mono#18061
Context: mono/mono#18064
Context: mono/mono#18106
Context: mono/mono#18120
Context: mono/mono#18191
Context: mono/mono#18202
Context: mono/mono#18213
Context: mono/mono#18221
Context: mono/mono#18247
Context: mono/mono#18273
Context: mono/mono#18276
Context: mono/mono#18317
Context: mono/mono#18323
Context: mono/mono#18364
Context: mono/mono#18370
Context: mono/mono#18417
Context: mono/mono#18418
Context: mono/mono#18455
Context: mono/mono#18506
Context: mono/mono#18524
Context: mono/mono#18530
Context: mono/mono#18554
Context: mono/mono#18572
Context: mono/mono#18578
Context: mono/mono#18584
Context: mono/mono#18612
Context: mono/mono#18614
Context: mono/mono#18675
Context: mono/mono#18676
Context: mono/mono#18925
Context: mono/mono#19009
Context: https://issuetracker.unity3d.com/issues/unity-physics-collisions-do-not-work-and-errors-are-thrown-when-entering-play-mode
Context: https://xamarin.github.io/bugzilla-archives/20/20233/bug.html
jonpryor pushed a commit to xamarin/xamarin-android that referenced this issue Mar 15, 2020
@directhex @jonpryor
Bump to mono/mono@d90665a4 (#4219)
b448469 
Changes: mono/api-snapshot@53a841f...5b8247e

	$ git diff --shortstat 53a841ff...5b8247e2
	 9 files changed, 572 insertions(+), 13 deletions(-)

Changes: mono/corefx@1cdb9c2...7c9e215

	$ git diff --shortstat 1cdb9c20...7c9e2158
	 24 files changed, 2393 insertions(+), 396 deletions(-)

Changes: dotnet/cecil@a6a7f5c...8021f3f

	$ git diff --shortstat a6a7f5c0...8021f3fb
	 16 files changed, 98 insertions(+), 25 deletions(-)

Changes: dotnet/linker@e8d054b...e1c7a72

	$ git diff --shortstat e8d054bf...e1c7a729
	 220 files changed, 9758 insertions(+), 3165 deletions(-)

Changes: mono/mono@2ff8988...d90665a

	$ git diff --shortstat 2ff89885...d90665a4
	 612 files changed, 20193 insertions(+), 10239 deletions(-)

Context: mono/mono#9726
Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1048838
Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1050615
Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1069059
Context: mono/mono#10643
Context: mono/mono#10651
Context: mono/mono#12022
Context: mono/mono#12995
Context: mono/mono#15612
Context: mono/mono#16513
Context: mono/mono#16588
Context: mono/mono#16778
Context: mono/mono#16969
Context: mono/mono#17140
Context: mono/mono#17601
Context: mono/mono#17869
Context: mono/mono#17878
Context: mono/mono#17916
Context: mono/mono#17926
Context: mono/mono#17980
Context: mono/mono#18006
Context: mono/mono#18019
Context: mono/mono#18020
Context: mono/mono#18030
Context: mono/mono#18061
Context: mono/mono#18064
Context: mono/mono#18106
Context: mono/mono#18120
Context: mono/mono#18191
Context: mono/mono#18202
Context: mono/mono#18213
Context: mono/mono#18221
Context: mono/mono#18247
Context: mono/mono#18273
Context: mono/mono#18276
Context: mono/mono#18317
Context: mono/mono#18323
Context: mono/mono#18364
Context: mono/mono#18370
Context: mono/mono#18417
Context: mono/mono#18418
Context: mono/mono#18455
Context: mono/mono#18506
Context: mono/mono#18524
Context: mono/mono#18530
Context: mono/mono#18554
Context: mono/mono#18572
Context: mono/mono#18578
Context: mono/mono#18584
Context: mono/mono#18612
Context: mono/mono#18614
Context: mono/mono#18675
Context: mono/mono#18676
Context: mono/mono#18925
Context: mono/mono#19009
Context: https://issuetracker.unity3d.com/issues/unity-physics-collisions-do-not-work-and-errors-are-thrown-when-entering-play-mode
Context: https://xamarin.github.io/bugzilla-archives/20/20233/bug.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-Runtime: Performance To track performance improvement related issues task
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[jit] Implement fast TLS access in gshared methods. vargaz/mono
2 participants
@vargaz @fanyang-mono