libxmlb icon indicating copy to clipboard operation
libxmlb copied to clipboard

Possibly inline a number of internals

Open chergert opened this issue 1 year ago • 3 comments

It looks like LTO isn't doing such a great job of interning a number of things that I think are worth seeing inlined. Not because these functions themselves show up on profiles a whole lot (but they do show up) but instead that the reduce the amount of optimization that can surround them in tight loops.

These aren't any panacea or anything, but it does help ensure that the compiler knows what is going on across the function call and perhaps reorder things a bit more.

  • Hoist a bunch of xb-opcode.c into xb-opcode-private.h
  • Hoist a bunch (all) of xb-silo-node.c into xb-silo-node-private.h

And of course bunch of that can be const and G_GNUC_PURE in the process.

The structures here are already exposed in the private headers and so static inlineing the helpers that are used everywhere makes sense to me.

While I'm here, I also wondered about perhaps changing how xb_machine does GError. Turns out we hit the failure path in xb_machine_check_two_args() quite a bit. Is it essential to propagate opcode level error tracking?

Anyway, wanted to get your thoughts before I spend too much time on it.

chergert avatar Aug 01 '23 01:08 chergert

Another thing we might want to look at is creating a form of silo_query_with_root() which takes an XbQuery. Even after ensuring that calling code uses a lot of cached XbQuery, this one is still easily >= 1% of CPU time in xb_query_new() alone and extra .3% to finalize the just created XbQuery (which would turn into a decref of a cached XbQuery in most cases).

The primary use here is via xb_node_query_attr(), of which that API rquires an xpath string.

chergert avatar Aug 01 '23 22:08 chergert

Hi. I'm on holiday with limited internet. I'll get to this next week. I'm back Wednesday. Thanks!

hughsie avatar Aug 02 '23 11:08 hughsie

No worries! And enjoy your well deserved holiday :)

chergert avatar Aug 02 '23 17:08 chergert