If we are using libffi to trampoline then there is opportunity to cache
the ffi_call_plan introduced in newer libffi releases. A rather contrived
test introduced in the previous commit includes a basic performance test
that can compare between versions.
For my system, the average call time dropped from 200ns/call to 145ns/call
for cached GClosure. Since the previous version did no caching, the
uncached GClosure use was also just slightly slower around 205ns/call. For
the cached form (since it can reuse a global plan cache via GMutex) it
drops to 180ns/call. So that is 27.5% savings and a 10% savings in runtime
duration.
Closes: #4002
Co-authored-by: Philip Withnall <pwithnall@gnome.org>
The goal here is to get a baseline for upcoming performance work
to use the cached libffi plans rather than generating a new plan
on every pass through libffi.
g_value_reset() returns the GValue passed in, so ownership of the
storage stays with the caller. Annotate the return value as borrowed
to prevent introspection consumers from treating it as newly owned.
msys2-clang64 noticed that the `spacing` variable was written to but
never read from. Indeed, this option has been a no-op since I
accidentally/deliberately broke it in commit
98e6ac788e.
Drop it, because nobody has complained about it breaking since 2022, and
I’m not sure about what the use case for it originally was meant to be.
See https://gitlab.gnome.org/pwithnall/glib/-/jobs/7171235
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
The act of initialising a class requires acquiring the class_init
recursive mutex alongside the type read/write mutex, to avoid the case
when one thread calls the type registration function on type B in the
middle of the class initialisation of type A—for instance, to add a
property of type B.
The initial implementation of GTypeInterface allowed adding interfaces
to a GType after the class had already been initialised, which leads to
a deadlock if, in the case outlined above, type B also adds an
interface. See issue #3892 for an example.
Luckily for us, we have disabled adding interfaces to types after their
class has been initialised: see commits c2055f22 and 545b4444.
Now that 14 years have passed, we can remove the class_init_rec_mutex
lock acquisition when adding an interface to a static or a dynamic
types, and when getting the default interface vtable.
See: #541
clang 23 added -Wunused-but-set-global, which fires on the
type_name##_parent_class variable that _G_DEFINE_TYPE_EXTENDED_BEGIN_PRE
emits whenever a type never chains up to its parent:
error: variable 'my_thing_parent_class' set but not used
[-Werror,-Wunused-but-set-global]
Projects that promote the unused-but-set group to an error (pango, for
one) now fail to build with clang 23, and they cannot fix it on their
side because the variable comes from this macro.
Annotate it G_GNUC_UNUSED, the same treatment the adjacent
get_instance_private helper already gets.
Signed-off-by: Sunil Dora <sunilkumar.dora@windriver.com>
Fixes: #4038
This path function is deprecated because it doesn't work properly if you
are using paths. glib-mkenums is using it "properly" in the sense that
it is not passing it any paths, but on the other hand, passing non-paths
to a path function is pretty silly. You can see why the function is
deprecated.
Embarassingly, my replacement implementation is AI-generated, although I
have simplified it to be less robust than the AI proposed. Why did I
intentionally make the function less robust? Because it's only used in
one place, and doesn't need to be able to handle any possible number of
arguments.
Clang-tidy has a check to prohibit identifiers that are reserved for the
compiler. Those are identifiers that start with:
- an underscore plus an uppercase letter
- two underscores
- an underscore, and are in the global scope
https://clang.llvm.org/extra/clang-tidy/checks/bugprone/reserved-identifier.html
Many public GLib macros define variables starting with two underscores,
which is technically not allowed. This is probably usually harmless, but
it causes false positives in the abovementioned clang-tidy check when
consumers of GLib use macros like G_VALUE_HOLDS_POINTER or
g_str_has_suffix.
This commit replaces all such names with one-underscore names, which are
not reserved because they're not in the global scope.
Here's the incantation I used to discover which macros needed editing:
git grep -P '\b__(?!builtin|atomic|sync|asm|volatile|pragma)[a-z].*\\$' \
-- '*.h' | grep -v valgrind.h
This generates a manageable number of false positives. It also fails to
find identifiers on the last line of a macro, but those are generally
already mentioned elsewhere in the macro.
I left valgrind.h untouched.
... added since 2.74. 2.74 was selected arbitrarily.
When a Since annotation is written inline for an enum member, g-ir won't
parse it, they need dedicated doc blocks.
g_type_check_is_value_type and g_type_test_flags both read global memory,
so can't be const. They only seem to write for the purpose of a lock, which
they undo at the end, so maybe we can get away with G_GNUC_PURE here.
Bug: https://gitlab.gnome.org/GNOME/glib/-/work_items/3984
Signed-off-by: Sam James <sam@gentoo.org>
g_type_ensure has a large comment saying _get_type shouldn't be
marked with G_GNUC_CONST because it has side-effects on the first
run, but it's OK if you consistently call g_type_ensure first.
Unfortunately, this isn't accurate. GCC trunk (observed at r17-1725-ge259a182e1f100)
now analyses points-to information more effectively and propagates its
findings, so it sees the function modifies some global state, concludes
that can't be possible (because of the const attribute), and deletes
the stores.
pure unfortunately is unsuitable for *_get_type too as it has a side-effect
(writes to global memory).
Mesa had a similar issue which I fixed recently [0]. glib and glibmm likely
want the (not-yet-implemented) 'idempotent' attribute instead [1]. In fact,
this exact problem is what motivated that request! :)
ebassi mentions an alternative to the current setup as well, using
atomics [2].
This problem has gone back many years but the compiler has finally
fully outsmarted us here, so give in.
[0] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/42361
[1] https://gcc.gnu.org/PR32911
[2] https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4249#note_2583110
Fixes: https://gitlab.gnome.org/GNOME/glib/-/work_items/3984
Signed-off-by: Sam James <sam@gentoo.org>
This commit extends the generic marshaller to support parameter and
return types that are custom GTypeInstance. This can be useful for
situations where writing a custom marshaller is impractical or lacks the
necessary plumbing to do so, such as <closure/> in GtkExpression defined
from UI files.
Fixes: #3954
Commit 18fbb182e8 changed the error
message for some conflicting GObject pspec flags, but the tests were not
updated to follow it. This wasn’t noticed because these tests are only
run with `-m thorough` as they’re a bit slow, so they only run weekly.
But we haven’t been noticing weekly CI run failures because the GitLab
issue-bot to report those failures has been broken for a long time. 😩
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Just as all the other checks are disabled by virtue of
`g_return_val_if_fail()` being a no-op with `G_DISABLE_CHECKS`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
This patch fixes several const qualifiers by adjusting where required.
Since glibc-2.43 and ISO C23, the functions bsearch, memchr, strchr,
strpbrk, strrchr, strstr, wcschr, wcspbrk, wcsrchr, wcsstr and wmemchr
that return pointers into their input arrays now have definitions as
macros that return a pointer to a const-qualified type when the input
argument is a pointer to a const-qualified type.
Signed-off-by: Rudi Heitbaum <rudi@heitbaum.com>
"default" can be confusing when comparing GValue to GParamSpec.
As a result, we chose to use "initial" as a term instead of "default" in
GTK. See: https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/9461
This updates the docs to match that decision.
Test objects-refcount1 is built with -Werror (see !3139).
This causes a compilation failure on clang-cl:
glib/gobject/tests/objects-refcount1.c(64,7):
error: cast from 'void (*)(GTestClass *)' (aka 'void (*)(struct _GTestClass *)')
to 'GClassInitFunc' (aka 'void (*)(void *, void *)') converts to incompatible
function type [-Werror,-Wcast-function-type-mismatch]
64 | (GClassInitFunc) my_test_class_init,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
All the extent warnings in this library have been fixed, so we can now
enable the warning flag to prevent new ones being added in future.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3527
Use `size_t` for array indexing.
This fixes a `-Wshorten-64-to-32` warning, but it would have been
practically impossible to hit this as a bug in practice.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3527
The `GObjectClass.n_construct_properties` member is of type `size_t`,
which is correct. However, it’s passed to `GObjectClass.constructor()`,
which takes an `unsigned int`. So in practice its maximum value is
`UINT_MAX`.
Add documentation and an assertion to check that, since it’s public API
and can’t be changed.
In practice, we don’t expect any code to come anywhere near this, but
it’s good to be correct (and this silences a `-Wshorten-64-to-32`
warning).
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3527
This was too wide, which was harmless but unnecessary.
Confusingly, signal IDs are `unsigned int`, but signal *connection* IDs
are `unsigned long`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3527
For historic reasons (probably to try and support enums with more than
32 values), enums and flags are stored in the `long` field of a `GValue`.
This is fine, but it triggers a load of `-Wshorten-64-to-32` warnings.
Add some explicit casts to silence those warnings. I don’t think we can
do any better for the implementation anyway. This doesn’t change the
behaviour from before, it just makes the implicit casts explicit.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3527
Fix an array iteration to correctly use `size_t`, and add an assertion
to check the number of arguments passed to libffi is not too many to
overflow its argument (which is, unfortunately, an `unsigned int` rather
than a `size_t`).
Neither of these are real bugs, but fixing the warnings means we can
enable the warning flag permanently to catch actual bugs in future.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3527
As spotted by `-Wshorten-64-to-32`, the signal connection IDs were being
stored in an `int` rather than a `long`, which could truncate them if
the application has a lot of signals connected.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3527
Instead of looking up for an handler and remove it later, just do it in
one operation, avoiding an unneeded extra-lookup every time we
disconnect an handler
We may lookup an handler by its ID or by a closure, never by both.
Thus there's no reason to have a function with two cases when they are
very different ones.
So let's just split it in two cases, allowing some simplification
This may currently happening during the gsettings binding destruction
and in this case we end up looking up for a NULL closure with a 0
handler ID for no reason, while we can just do an early return
