Remove reliance on static initializers for registration #359

Open
opened 2024-05-29 12:50:49 +00:00 by delroth · 5 comments
Member

A few places in Lix rely on static initializers to register various ops or backends. This is generally a bad idea: static initializers have undefined execution ordering which can vary across builds or even sometimes across runs, they break inter-module dependency analysis which can lead to really weird behavior with static .a builds, they break a whole bunch of static analysis / unused code detection, etc.

They also happen to be global and impossible to be made not-global, which could impair plans for a better liblix.

Let's instead switch to explicit registration of those ops / backends / ... as part of an "init" function/method. This will keep them as global for now - this proposed change is neutral on that dimension. The old Register* structs used for static initializer registration will be kept in the short term as __attribute__((deprecated))for plugin compatibility, but this might be revisited if we decide not to care about plugin source compatibility.

Known places to update:

  • RegisterLegacyCommand (cl/1905)
  • RegisterCommand
  • GlobalConfig::Register
  • RegisterStoreImplementation (cl/1385)
  • RegisterPrimOp
  • Fix plugin-files setting documentation
A few places in Lix rely on static initializers to register various ops or backends. This is generally a bad idea: static initializers have undefined execution ordering which can vary across builds or even sometimes across runs, they break inter-module dependency analysis which can lead to really weird behavior with static .a builds, they break a whole bunch of static analysis / unused code detection, etc. They also happen to be global and impossible to be made not-global, which could impair plans for a better liblix. Let's instead switch to explicit registration of those ops / backends / ... as part of an "init" function/method. This will keep them as global for now - this proposed change is neutral on that dimension. The old `Register*` structs used for static initializer registration will be kept in the short term as `__attribute__((deprecated))`for plugin compatibility, but this might be revisited if we decide not to care about plugin source compatibility. Known places to update: - [ ] RegisterLegacyCommand (cl/1905) - [ ] RegisterCommand - [ ] GlobalConfig::Register - [x] RegisterStoreImplementation (cl/1385) - [ ] RegisterPrimOp - [ ] Fix `plugin-files` setting documentation
Member

The beginning for RegisterPrimOp removal is at https://gerrit.lix.systems/c/lix/+/1342

The beginning for `RegisterPrimOp` removal is at https://gerrit.lix.systems/c/lix/+/1342
jade added the
stability
label 2024-05-30 07:11:49 +00:00
Author
Member

Of the cases I've listed, I think GlobalConfig::Register will likely be the thorniest. It's being used across shared libraries, which means it's really not trivial to invert the dependency without introducing loops. Most likely it will need a fairly major refactor of the initialization API for all the sub libs in Lix.

The other ones seem self contained to a single shared module so they should be easier.

Of the cases I've listed, I think `GlobalConfig::Register` will likely be the thorniest. It's being used across shared libraries, which means it's really not trivial to invert the dependency without introducing loops. Most likely it will need a fairly major refactor of the initialization API for all the sub libs in Lix. The other ones seem self contained to a single shared module so they should be easier.
Author
Member

cl/1385 should take care of RegisterStoreImplementation

cl/1385 should take care of `RegisterStoreImplementation`
Member

This issue was mentioned on Gerrit on the following CLs:

  • commit message in cl/1385 ("libstore: remove static initializers for Store registrations")
  • commit message in cl/1905 ("Remove static initializers for RegisterLegacyCommand")
  • commit message in cl/1904 ("liblegacy: init")
<!-- GERRIT_LINKBOT: {"cls": [{"backlink": "https://gerrit.lix.systems/c/lix/+/1385", "number": 1385, "kind": "commit message"}, {"backlink": "https://gerrit.lix.systems/c/lix/+/1905", "number": 1905, "kind": "commit message"}, {"backlink": "https://gerrit.lix.systems/c/lix/+/1904", "number": 1904, "kind": "commit message"}], "cl_meta": {"1385": {"change_title": "libstore: remove static initializers for Store registrations"}, "1905": {"change_title": "Remove static initializers for `RegisterLegacyCommand`"}, "1904": {"change_title": "liblegacy: init"}}} --> This issue was mentioned on Gerrit on the following CLs: * commit message in [cl/1385](https://gerrit.lix.systems/c/lix/+/1385) ("libstore: remove static initializers for Store registrations") * commit message in [cl/1905](https://gerrit.lix.systems/c/lix/+/1905) ("Remove static initializers for `RegisterLegacyCommand`") * commit message in [cl/1904](https://gerrit.lix.systems/c/lix/+/1904) ("liblegacy: init")
Owner

Removed RegisterLegacyCommand in https://gerrit.lix.systems/c/lix/+/1905

Removed `RegisterLegacyCommand` in https://gerrit.lix.systems/c/lix/+/1905
Sign in to join this conversation.
No milestone
No project
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lix-project/lix#359
No description provided.