Skip to content

New API for supporting mut aliasing safety#1753

Open
Indra-db wants to merge 46 commits intoSanderMertens:masterfrom
Indra-db:table_column_locks_cr_locks
Open

New API for supporting mut aliasing safety#1753
Indra-db wants to merge 46 commits intoSanderMertens:masterfrom
Indra-db:table_column_locks_cr_locks

Conversation

@Indra-db
Copy link
Contributor

@Indra-db Indra-db commented Aug 17, 2025

This PR introduces:

  • locking for either component records (sparse components) or table-column locks (non-sparse components).
  • separate per-thread / per-stage locking for table columns.
  • everything gated by the FLECS_MUT_ALIAS_LOCKS define, so there is no performance impact when it’s not enabled.
  • build tests and unit tests for the behaviour.

End users of this internal API must use it themselves and panic when the mut-alias guarantee is violated.

ecs_lock_target_t holds information about where the lock should be applied. For sparse components/IDs (sparse and/or non-fragmenting) it will use the component_record for locking. For table-column components it will lock the individual column.

ecs_get_id and ecs_get_mut_id have been split, it now calls ecs_record_get_id / ecs_record_get_mut_id which starts from an entity record. The main purpose is that the ecs_record_get_id returns an ecs_get_ptr struct instead of just a void*, which contains the lock target information. The second purpose of this change is that stop the Rust bindings for flecs from maintaining their own get functions (a maintenance hell). Flecs-rust retrieves the record once, then uses its get callback to retrieve component pointers. I believe the same approach could be used for the C++ get callback.

Changes to ecs_get_id and ecs_get_mut_id need to be benchmarked by Sander.

Changing ecs_get_id and ecs_get_mut_id means a lot more scrutiny has to be applied to this PR. This could cause performance regressions that depend on compiler-specific inlining behavior.
I'll need to test this myself — not sure if I'll have time before the next release, but I'll try.

@Indra-db
Copy link
Contributor Author

ci test, I expect a failure somewhere, I cannot run tests locally due to bake being problematic (see discord)

@Indra-db
Copy link
Contributor Author

all tests passing 🥳

@Reddy-dev
Copy link
Contributor

Reddy-dev commented Aug 21, 2025

ci test, I expect a failure somewhere, I cannot run tests locally due to bake being problematic (see discord)

@Indra-db could you link the message, I think i also have similar issues

@Indra-db
Copy link
Contributor Author

@Reddy-dev

Tldr, Sander told me to use bake run as opposed to bake test which should fix the ouput

@Indra-db
Copy link
Contributor Author

@Reddy-dev this was my thread where I was dumping my experience (which I closed)
https://discord.com/channels/633826290415435777/1406452408149741770/1406998533449580585

@Indra-db
Copy link
Contributor Author

update to latest main

@Indra-db Indra-db changed the title table column & component record safety locks table column & component record mut alias locks Sep 23, 2025
@Indra-db Indra-db changed the title table column & component record mut alias locks table column & component record mut alias locks internal API Sep 23, 2025
@SanderMertens SanderMertens changed the title table column & component record mut alias locks internal API New API for supporting mut aliasing safety Sep 23, 2025
@Indra-db Indra-db force-pushed the table_column_locks_cr_locks branch from d4af765 to 82365c3 Compare November 17, 2025 02:30
@SanderMertens
Copy link
Owner

I'm still seeing performance regressions when comparing with latest master:

Screenshot 2025-12-06 at 2 44 11 PM

I think it's better to separate out the implementation into its own functions so there's no risk of regressions.

(Note: benchmarks were done without the FLECS_MUT_ALIAS_LOCKS flag on a regular Flecs master)

commit b3b50fc
Author: Indradb <60851042+Indra-db@users.noreply.github.com>
Date:   Sun Sep 14 01:15:40 2025 +0900

    unused parameter fix.

commit f23337b
Author: Indradb <60851042+Indra-db@users.noreply.github.com>
Date:   Sat Sep 13 18:53:24 2025 +0900

    add tests to verify safety info is correctly initialized

commit 87b9a1f
Merge: 936aba1 98d3437
Author: Indradb <60851042+Indra-db@users.noreply.github.com>
Date:   Sat Sep 13 18:15:14 2025 +0900

    update to latest main

commit 936aba1
Author: Indradb <60851042+Indra-db@users.noreply.github.com>
Date:   Sun Aug 17 17:53:25 2025 +0900

    remove define

commit 0518a78
Author: Indradb <60851042+Indra-db@users.noreply.github.com>
Date:   Sun Aug 17 15:47:34 2025 +0900

    FLECS_SI_INIT macro: cast column index to i16

commit 9a040b5
Author: Indradb <60851042+Indra-db@users.noreply.github.com>
Date:   Sun Aug 17 15:24:35 2025 +0900

    forward declare ecs_safety_info_t

commit e70778f
Author: Indradb <60851042+Indra-db@users.noreply.github.com>
Date:   Sun Aug 17 12:54:12 2025 +0900

    update distr files

commit 70240c9
Author: Indradb <60851042+Indra-db@users.noreply.github.com>
Date:   Sun Aug 17 12:45:17 2025 +0900

    fix doc mention

commit b60b666
Author: Indradb <60851042+Indra-db@users.noreply.github.com>
Date:   Sun Aug 17 10:05:40 2025 +0900

    remove entity argument from get_mut_id_from_record

commit a05c550
Author: Indradb <60851042+Indra-db@users.noreply.github.com>
Date:   Sat Aug 16 22:57:48 2025 +0900

    introduce safety info + ecs_get_id_from_record & refactor ecs_get_id methods to use it
…ty_info_t to ecs_lock_target_t, merge typedef def
@Indra-db Indra-db force-pushed the table_column_locks_cr_locks branch from 8f5ff7d to cc8a6da Compare December 24, 2025 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments