Core: Add initial architecture for first-class Object types. Optimize is_class

[Ivorforce]

• Requires Use non-recursive mutex for StringName, improving performance #105192 (for refactor reasons).
• Follow-up: Expand GDType to cover GDExtension types as well #106016

This is a first step towards adding first-class types to Godot.
This is better explained in the proposal: godotengine/godot-proposals#12323.

After this PR, we can slowly build on top of it to move things from ClassDB into GDType.

Implementation

I piggy-back off the previous get_class_namev scaffolding, using the same mechanism to use and assign GDType * instead. For this reason, the implementation is safe and battle tested before being implemented.

In particular, each Object owns a const GDType * to its own type. The type exposes methods for getting type-related information.

I make use of this new construct by optimizing is_class. Currently, it is implemented as a virtual function, which uses GDCLASS to check against static StringName singletons:

godot/core/object/object.h

Lines 432 to 437 in e37c626

	virtual bool is_class(const String &p_class) const override { \
	if (_get_extension() && _get_extension()->is_class(p_class)) { \
	return true; \
	} \
	return (p_class == (#m_class)) ? true : m_inherits::is_class(p_class); \
	} \

This is slow, not just because various checks were repeatedly called for each subclass, but also because the elements weren't local in memory, and each had to be put into cache separately.
Instead, I create a constant LocalVector of the hierarchy for each type, and simply loop through it to find the queried class name.
(This would be much faster if the argument was a StringName instead of String, but that will happen sooner or later anyway as the code is refactored and optimized)

Benchmark

I benchmarked a ~3x improvement of is_class. The improvement would likely be better outside benchmarks because there are fewer cache misses.

	// Setup
	Node3D *node = memnew(Node3D);
	String class_name = "Node2D";

	auto t0 = std::chrono::high_resolution_clock::now();
	for (int i = 0; i < 500000000; i ++) {
		// Test
		node->is_class(class_name);
	}
	auto t1 = std::chrono::high_resolution_clock::now();
	std::cout << std::chrono::duration_cast<std::chrono::milliseconds>(t1 - t0).count() << "ms\n";

This printed:

• 7845ms on master
• 2662ms on this PR

[lawnjelly]

Alternative just off top of my head, don't know if this is doable:

• Each new class type registers itself with Manager using classname and gets allotted an increasing ID
• Ask the Manager not the class whether an object is / derived from classname
• Manager creates bitfields for each class, if there are 200 classes in all, each bitfield is 200 bits, sets 1 if a member of that ID
• For lookup, look up the ID in hashtable for the classname, then check the bitfield of the Object that is being queried.

That should be a hashtable lookup, and a bit check.

Essentially I'm wondering if the vector can be replaced by a bitfield, and then check by ID instead of iterating through the vector. 🤔

[Ivorforce]

Alternative just off top of my head, don't know if this is doable:

• Each new class type registers itself with Manager using classname and gets allotted an increasing ID
• Ask the Manager not the class whether an object is / derived from classname
• Manager creates bitfields for each class, if there are 200 classes in all, each bitfield is 200 bits, sets 1 if a member of that ID
• For lookup, look up the ID in hashtable for the classname, then check the bitfield of the Object that is being queried.
  That should be a hashtable lookup, and a bit check.

Essentially I'm wondering if the vector can be replaced by a bitfield, and then check by ID instead of iterating through the vector. 🤔

BitField is unfortunately limited to the number of bits in whatever primitive you choose for it. A BitField<uint64_t> can only store 64 independent states (i.e. hold 64 classes). Since GDExtensions can register GDCLASS as well, it would need to be variable length (i.e. heap allocated).
There are around 1700 GDCLASS in Godot alone, so it would necessitate this 'variable' BitField to be at least 212 bytes long. This wouldn't be any better than the ~32 bytes currently needed for a class with 3 superclasses (e.g. Object -> Node -> Node3D -> Path3D).

Indirection is also a notable concern: The current system identifies each class uniquely with a constexpr void *. To check for subclasses, you need 3 indirections (object->gdtype->identifier_hierarchy->void * identifiers) (notably, moving is_class_ptr to this system is planned for a follow-up PR). With a Manager system, you'd need several more: object->gdtype->dynamic_bitfield + manager->hashmap->...->dynamic_bitfield.

The main reason I want to get rid of this extensive ClassDB / manager system in the first place is to remove these layers of indirection, and just let objects know their own type as directly as possible :)

[lawnjelly]

BitField is unfortunately limited to the number of bits in whatever primitive you choose for it. A BitField<uint64_t> can only store 64 independent states (i.e. hold 64 classes). Since GDExtensions can register GDCLASS as well, it would need to be variable length (i.e. heap allocated).

Yup, sure, e.g. BitfieldDynamic (we have one in 3.x).

There are around 1700 GDCLASS in Godot alone, so it would necessitate this 'variable' BitField to be at least 212 bytes long. This wouldn't be any better than the ~32 bytes currently needed for a class with 3 superclasses (e.g. Object -> Node -> Node3D -> Path3D).

Ah maybe that would be the clincher, I didn't realize we had so many GDCLASSes. 😁

But maybe as we mentioned a while back, we could have a parallel simple faster bit check available for classes derived from Spatial, CanvasItem etc, so it would get the best of both worlds.

[Ivorforce]

BitField is unfortunately limited to the number of bits in whatever primitive you choose for it. A BitField<uint64_t> can only store 64 independent states (i.e. hold 64 classes). Since GDExtensions can register GDCLASS as well, it would need to be variable length (i.e. heap allocated).

Yup, sure, e.g. BitfieldDynamic (we have one in 3.x).

There are around 1700 GDCLASS in Godot alone, so it would necessitate this 'variable' BitField to be at least 212 bytes long. This wouldn't be any better than the ~32 bytes currently needed for a class with 3 superclasses (e.g. Object -> Node -> Node3D -> Path3D).

Ah maybe that would be the clincher, I didn't realize we had so many GDCLASSes. 😁

But maybe as we mentioned a while back, we could have a parallel simple faster bit check available for classes derived from Spatial, CanvasItem etc, so it would get the best of both worlds.

Agreed, that's definitely worth checking for performance benefits!

[Ivorforce]

Just a little update, I was able to use ClassInfo directly in Object, instead of introducing a new GDType. This ended up being a much more fully featured implementation. However, it also require some fixes to main registering order.

In the end, it makes more sense to build around and optimize existing systems than to introduce a new one next to it. I'll probably close this pull request, open the other implementation as a draft, and try to slowly build towards it with the changes mentioned above split into smaller, incremental packages.
#106646 and #106775 are good first steps forwards, in any case.

[Shadows-of-Fire]

Wait, so, what is is_class exactly (i.e. what are we trying to optimize here)? The name seems... not great, to say the least, and the header doesn't have a doc.

Is it an instanceof operation? i.e. this->is_class("Node") === this instanceof Node?

Edit: I believe the answer is "yes", at which point this is a solid way to implement this operation.

[Ivorforce]

Yes, is_class is oddly named / designed. It already exists and should probably be revised at some point.

[Shadows-of-Fire]

In the theme of promoting the type-first architecture, maybe this PR should introduce Object::is_subtype_of(GDType const&) (optionally with an is_subtype_of(String) overload that resolves the type and calls the other one).

I realize right now we do a lot of "pass the StringName around and do lookups with it", but we ought to be in the mindset of "Pass the GDType around and do stuff with that directly"

[Shadows-of-Fire]

Is there a point to have this at all rather than traversing the chain of superclasses? It seems like the issue is (potentially) memory locality, but seeing as StringName is interned "somewhere else" (meaning the accesses to evaluate == will need to reach that "else"), the space used by having a Vector on each GDType might not be worth it.

[Ivorforce]

I benchmarked a 3x speed improvement over the old is_class (hot access, cold should be even better). Having a Vector<StringName> for each GDType is not a big cost, so I'd argue it's worth it.
However, I've been second guessing adding this in the first "GDType" draft so I might revise / supersede the PR soon and propose this change later.

[Shadows-of-Fire]

The thing is the old is_class impl is so comically bad that we might get the same 3x uplift (or very very close to it) simply by traversing the links here. If you already have the benchmark setup, I'd be willing to bet that the pointer traversal is really close to this one without the extra vector.

FWIW the OpenJDK implementation for this problem does a pointer traversal through the supertypes, which makes me think that approach is very viable.

[Ivorforce]

The thing is the old is_class impl is so comically bad that we might get the same 3x uplift (or very very close to it) simply by traversing the links here.

Yea, that's definitely a possibility. It's a good reason to propose this change separately (if at all) from working towards first-class types.

[Shadows-of-Fire]

Just a little update, I was able to use ClassInfo directly in Object, instead of introducing a new GDType.

I think there's value in both approaches there. Migrating to a new GDType forces re-assessment of the structures used by ClassInfo to see if they are really the best fit. Reusing ClassInfo gets us some familiarity due to existing systems. It's hard to say what will work best here.

[Ivorforce]

Just a little update, I was able to use ClassInfo directly in Object, instead of introducing a new GDType.

I think there's value in both approaches there. Migrating to a new GDType forces re-assessment of the structures used by ClassInfo to see if they are really the best fit. Reusing ClassInfo gets us some familiarity due to existing systems. It's hard to say what will work best here.

Re-assessing ClassInfo is probably a worthy goal, but we can approach it separately and make incremental improvements, rather than trying to replace it with a new system entirely. I'm a believer in refactoring over rewriting, most times anyway :)

[Shadows-of-Fire]

Incremental improvements tend to be the way to go. Though, on the merits, I think GDType is a better name, and moving to it (from ClassInfo) acts as a forcing function for that reassessment and as a forcing function for something I mentioned in one of the earlier threads - "Pass the GDType around and do stuff with that directly".

[Ivorforce]

Discussed in the Core meeting. We want to go ahead with adding a new GDType rather than adapting the existing ClassInfo from ClassDB. Functionality will slowly be moved over to GDType, redesigning when needed.

[dsnopek]

Thanks! This looks good to me, and should make a good base for further improvements :-)

[Repiteo]

While part of me still eventually hopes for an even more fundamental system utilizing type_traits, we've gotta start somewhere, and this is one hell of a way to start 👍

[Repiteo]

Thanks!