[Tizen] Run single thread for processing all GLib sources #25762

arkq · 2023-03-21T09:51:53Z

Problem

Currently Tizen platform spawns a new thread every time an async Tizen API call is about to be made. This is required to make sure that Tizen API internally will attach IO sources to correct context - context which is processed by Matter event loop. A dedicated Matter event loop is required, because it is possible, that Tizen application will not run default GLib main event loop.

This PR is a first step for adding proper matter stack locking when processing GLib-based Tizen events.

Changes

run Matter GLib main event loop in a dedicated thread started during Matter stack initialization
synchronically invoke all async Tizen API calls on the Matter GLib context (so it will be processed by our dedicated thread)
removed MainLoop class

Testing

Tested locally by performing BLE commissioning Linux-Tizen, Tizen-Linux and Tizen-Tizen.
CI will test network commissioning.

github-actions · 2023-03-21T10:06:01Z

PR #25762: Size comparison from f994dae to 6adb6e4

Decreases (1 build for cc32xx)

platform	target	config	section	`f994dae`	`6adb6e4`	change	% change
cc32xx	lock	CC3235SF_LAUNCHXL	.debug_info	20248023	20248022	-1	-0.0

Full report (1 build for cc32xx)

platform	target	config	section	`f994dae`	`6adb6e4`	change	% change
cc32xx	lock	CC3235SF_LAUNCHXL		0	0	0	0.0
			(read only)	645601	645601	0	0.0
			(read/write)	203848	203848	0	0.0
			.ARM.attributes	44	44	0	0.0
			.ARM.exidx	8	8	0	0.0
			.bss	197248	197248	0	0.0
			.comment	194	194	0	0.0
			.data	1480	1480	0	0.0
			.debug_abbrev	930292	930292	0	0.0
			.debug_aranges	87400	87400	0	0.0
			.debug_frame	300320	300320	0	0.0
			.debug_info	20248023	20248022	-1	-0.0
			.debug_line	`2661345`	`2661345`	0	0.0
			.debug_loc	2805489	2805489	0	0.0
			.debug_ranges	283264	283264	0	0.0
			.debug_str	3027174	3027174	0	0.0
			.ramVecs	780	780	0	0.0
			.resetVecs	64	64	0	0.0
			.rodata	105993	105993	0	0.0
			.shstrtab	232	232	0	0.0
			.stab	204	204	0	0.0
			.stabstr	441	441	0	0.0
			.stack	2048	2048	0	0.0
			.strtab	380421	380421	0	0.0
			.symtab	257408	257408	0	0.0
			.text	537488	537488	0	0.0

github-actions · 2023-03-26T08:06:54Z

PR #25762: Size comparison from ec2ad41 to 6ff2e19

Increases (1 build for cc32xx)

platform	target	config	section	`ec2ad41`	`6ff2e19`	change	% change
cc32xx	lock	CC3235SF_LAUNCHXL	.debug_info	20309905	20309906	1	0.0

Full report (1 build for cc32xx)

platform	target	config	section	`ec2ad41`	`6ff2e19`	change
cc32xx	lock	CC3235SF_LAUNCHXL		0	0	0
			(read only)	645745	645745	0
			(read/write)	203848	203848	0
			.ARM.attributes	44	44	0
			.ARM.exidx	8	8	0
			.bss	197248	197248	0
			.comment	194	194	0
			.data	1480	1480	0
			.debug_abbrev	933102	933102	0
			.debug_aranges	87704	87704	0
			.debug_frame	301604	301604	0
			.debug_info	20309905	20309906	1
			.debug_line	2680953	2680953	0
			.debug_loc	2827596	2827596	0
			.debug_ranges	286376	286376	0
			.debug_str	3041114	3041114	0
			.ramVecs	780	780	0
			.resetVecs	64	64	0
			.rodata	105953	105953	0
			.shstrtab	232	232	0
			.stab	204	204	0
			.stabstr	441	441	0
			.stack	2048	2048	0
			.strtab	380555	380555	0
			.symtab	257456	257456	0
			.text	537672	537672	0

src/platform/Tizen/BLEManagerImpl.h

github-actions · 2023-03-30T08:01:03Z

PR #25762: Size comparison from a27bc8c to 6a2c8c9

Increases (1 build for cc32xx)

platform	target	config	section	`a27bc8c`	`6a2c8c9`	change	% change
cc32xx	lock	CC3235SF_LAUNCHXL	.debug_info	20310043	20310044	1	0.0

Full report (1 build for cc32xx)

platform	target	config	section	`a27bc8c`	`6a2c8c9`	change
cc32xx	lock	CC3235SF_LAUNCHXL		0	0	0
			(read only)	645769	645769	0
			(read/write)	203848	203848	0
			.ARM.attributes	44	44	0
			.ARM.exidx	8	8	0
			.bss	197248	197248	0
			.comment	194	194	0
			.data	1480	1480	0
			.debug_abbrev	933102	933102	0
			.debug_aranges	87704	87704	0
			.debug_frame	301596	301596	0
			.debug_info	20310043	20310044	1
			.debug_line	2681003	2681003	0
			.debug_loc	2827360	2827360	0
			.debug_ranges	286352	286352	0
			.debug_str	3041144	3041144	0
			.ramVecs	780	780	0
			.resetVecs	64	64	0
			.rodata	105953	105953	0
			.shstrtab	232	232	0
			.stab	204	204	0
			.stabstr	441	441	0
			.stack	2048	2048	0
			.strtab	380543	380543	0
			.symtab	257472	257472	0
			.text	537696	537696	0

github-actions · 2023-03-30T14:57:39Z

PR #25762: Size comparison from a27bc8c to 2ee6abb

Full report (1 build for cc32xx)

platform	target	config	section	`a27bc8c`	`2ee6abb`
cc32xx	lock	CC3235SF_LAUNCHXL		0	0
			(read only)	645769	645769
			(read/write)	203848	203848
			.ARM.attributes	44	44
			.ARM.exidx	8	8
			.bss	197248	197248
			.comment	194	194
			.data	1480	1480
			.debug_abbrev	933102	933102
			.debug_aranges	87704	87704
			.debug_frame	301596	301596
			.debug_info	20310043	20310043
			.debug_line	2681003	2681003
			.debug_loc	2827360	2827360
			.debug_ranges	286352	286352
			.debug_str	3041144	3041144
			.ramVecs	780	780
			.resetVecs	64	64
			.rodata	105953	105953
			.shstrtab	232	232
			.stab	204	204
			.stabstr	441	441
			.stack	2048	2048
			.strtab	380543	380543
			.symtab	257472	257472
			.text	537696	537696

dh79pyun

Looks good

hyunuktak · 2023-04-11T01:26:55Z

It looks good overall. and i have a question for you.
In the changed structure, is there an action for timeout if the call is not completed for the asynchronous function call?

arkq · 2023-04-11T07:13:18Z

In the changed structure, is there an action for timeout if the call is not completed for the asynchronous function call?

@hyunuktak , no, currently there is no such concept as timeout for GLibMatterContextInvokeSync() calls. This function is not async, but synchronous by design. The purpose of this function is to run given callback on GLib context, that's all. The next step for this refactoring (in another PR) will be proper locking of matter stack when running callback function. However, we are not sure whether it will be possible without duplicating glib internal functions. So, other possibility is that the dedicated glib thread will be removed. So, for internal calls the g_main_context_invoke_full() in the GLibMatterContextInvokeSync() will not switch threads at all - callback will be called right away. If one wants async processing it will have to be done separately, e.g. use glib timeout source (but not with g_idle_add() - this will use global main context, and we do not want that).

EDIT:
As for now I will convert this PR to draft, because some small fixes are required for that PR (I've discovered some issues when working on #25960)

github-actions · 2023-04-14T14:24:13Z

PR #25762: Size comparison from b42e176 to e6a495f

Decreases (1 build for cc32xx)

platform	target	config	section	`b42e176`	`e6a495f`	change	% change
cc32xx	lock	CC3235SF_LAUNCHXL	.debug_info	20326061	20326060	-1	-0.0

Full report (1 build for cc32xx)

platform	target	config	section	`b42e176`	`e6a495f`	change	% change
cc32xx	lock	CC3235SF_LAUNCHXL		0	0	0	0.0
			(read only)	643081	643081	0	0.0
			(read/write)	203848	203848	0	0.0
			.ARM.attributes	44	44	0	0.0
			.ARM.exidx	8	8	0	0.0
			.bss	197248	197248	0	0.0
			.comment	194	194	0	0.0
			.data	1480	1480	0	0.0
			.debug_abbrev	933224	933224	0	0.0
			.debug_aranges	87760	87760	0	0.0
			.debug_frame	302028	302028	0	0.0
			.debug_info	20326061	20326060	-1	-0.0
			.debug_line	`2687403`	`2687403`	0	0.0
			.debug_loc	2838361	2838361	0	0.0
			.debug_ranges	288040	288040	0	0.0
			.debug_str	3042379	3042379	0	0.0
			.ramVecs	780	780	0	0.0
			.resetVecs	64	64	0	0.0
			.rodata	104393	104393	0	0.0
			.shstrtab	232	232	0	0.0
			.stab	204	204	0	0.0
			.stabstr	441	441	0	0.0
			.stack	2048	2048	0	0.0
			.strtab	377626	377626	0	0.0
			.symtab	256832	256832	0	0.0
			.text	536568	536568	0	0.0

github-actions bot added platform tizen For Tizen platform labels Mar 21, 2023

arkq added 10 commits March 26, 2023 09:54

Remove unused timeout in MainLoop::AsyncRequest()

38215f9

Create dedicated glib thread during Matter stack initialization

bcbe38e

Use GLibMatterContextInvoke in BLE manager

6dfca68

Use GLibMatterContextInvoke in device scanner

fdad68a

Use GLibMatterContextInvoke in DNS-SD manager

7e061c9

Use GLibMatterContextInvoke in WiFi manager

77397cb

Remove MainLoop class

098a666

Use better name for synchronous invoke

f03ca96

Verify that thread default glib context is set

49d6474

Type safety for GLibMatterContextInvokeSynchronous

6ff2e19

arkq force-pushed the tizen-glib-one-thread-v2 branch from 6adb6e4 to 6ff2e19 Compare March 26, 2023 07:55

arkq marked this pull request as ready for review March 26, 2023 09:29

pullapprove bot requested review from amitnj, andy31415, anush-apple, bzbarsky-apple, carol-apple, chrisdecenzo, chshu, chulspro, cliffamzn, Damian-Nordic, dhrishi, electrocucaracha, franck-apple and gjc13 March 26, 2023 09:30

pullapprove bot requested review from sharadb-amazon, tcarmelveilleux, tecimovic, tehampson, turon, vijs, vivien-apple, woody-apple, xylophone21, younghak-hwang and yufengwangca March 26, 2023 09:30

pullapprove bot added the review - pending label Mar 26, 2023

jlatusek reviewed Mar 27, 2023

View reviewed changes

src/platform/Tizen/BLEManagerImpl.h Outdated Show resolved Hide resolved

Cleanup

1206d27

dh79pyun approved these changes Apr 10, 2023

View reviewed changes

arkq marked this pull request as draft April 11, 2023 07:16

arkq added 3 commits April 14, 2023 15:47

Merge branch 'master' into tizen-glib-one-thread-v2

c306c50

Align sync function name with convention

a842754

Wait for Matter glib event loop to start running

e6a495f

arkq force-pushed the tizen-glib-one-thread-v2 branch from 2ee6abb to e6a495f Compare April 14, 2023 14:07

arkq marked this pull request as ready for review April 14, 2023 14:09

andy31415 approved these changes Apr 14, 2023

View reviewed changes

andy31415 added the fast track label Apr 14, 2023

andy31415 merged commit a202465 into project-chip:master Apr 14, 2023

arkq deleted the tizen-glib-one-thread-v2 branch April 14, 2023 19:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Tizen] Run single thread for processing all GLib sources #25762

[Tizen] Run single thread for processing all GLib sources #25762

arkq commented Mar 21, 2023 •

edited

Loading

github-actions bot commented Mar 21, 2023

github-actions bot commented Mar 26, 2023

github-actions bot commented Mar 30, 2023

github-actions bot commented Mar 30, 2023

dh79pyun left a comment

hyunuktak commented Apr 11, 2023

arkq commented Apr 11, 2023 •

edited

Loading

github-actions bot commented Apr 14, 2023

[Tizen] Run single thread for processing all GLib sources #25762

[Tizen] Run single thread for processing all GLib sources #25762

Conversation

arkq commented Mar 21, 2023 • edited Loading

Problem

Changes

Testing

github-actions bot commented Mar 21, 2023

github-actions bot commented Mar 26, 2023

github-actions bot commented Mar 30, 2023

github-actions bot commented Mar 30, 2023

dh79pyun left a comment

Choose a reason for hiding this comment

hyunuktak commented Apr 11, 2023

arkq commented Apr 11, 2023 • edited Loading

github-actions bot commented Apr 14, 2023

arkq commented Mar 21, 2023 •

edited

Loading

arkq commented Apr 11, 2023 •

edited

Loading