wrap class methods eagerly instead of via __index #410

sneaky-potato · 2026-01-24T09:32:30Z

Methods are now wrapped once in lunatik_newclass(). This reduces metatable churn and avoids per-access closure allocation in performance-sensitive paths.

Tested performance with the following script

local linux = require("linux")
local data = require("data")

local d = data.new(1024)
local iterations = 1000000

local start_ns = linux.time()
for i = 1, iterations do
    d:getbyte(0)
end
local end_ns = linux.time()
local total_ns = end_ns - start_ns

local ns_per_call = total_ns / iterations

print("Total ns: " .. total_ns)
print("Ns per call: " .. ns_per_call)


local m_getbyte = d.getbyte 

start_ns = linux.time()
for i = 1, iterations do
    m_getbyte(d, 0)
end
end_ns = linux.time()
total_ns = end_ns - start_ns

ns_per_call = total_ns / iterations

print("Localized method total ns: " .. total_ns)
print("Localized method Ns per call: " .. ns_per_call)

master:

Total ns: 275151687
Ns per call: 275
Localized method total ns: 137531899
Localized method Ns per call: 137

this branch:

Total ns: 128897733
Ns per call: 128
Localized method total ns: 130847719
Localized method Ns per call: 130

Methods are now wrapped once in lunatik_newclass(). This reduces metatable churn and avoids per-access closure allocation in performance-sensitive paths. Signed-off-by: Ashwani Kumar Kamal <ashwanikamal.im421@gmail.com>

lunatik.h

lneto · 2026-01-24T12:03:35Z

lib/luacrypto_aead.c

 	{"decrypt", luacrypto_aead_decrypt},
 	{"__gc", lunatik_deleteobject},
 	{"__close", lunatik_closeobject},
-	{"__index", lunatik_monitorobject},


we will need another way to signalize where a class (actually, it would better to have it per object) need a monitor or not. Please notice, that on this patch all classes will use monitor, and we cannot do this. For instance, it probably will break RCU. I thought on having a bitfield carrying the object flags, including "shared" ("monitored") and "sleep". What do you think?

Right, I agree here. Currently I am pre wrapping all the objects which do not have index in the metatable. It would be better to check if an object really needs monitoring.
Couple of questions regarding structs lunatik_class_t and lunatik_object_t:

Why is there bool sleep in both lunatik_class_t and lunatik_object_t. Is it safe to assume that sleep in lunatik_class_t is redundant?

Lock type is unclear here (mutex vs spinlock), for instance, what happens if a wrong lock is taken?

I thought on having a bitfield carrying the object flags, including "shared" ("monitored") and "sleep". What do you think?

Yes, a flag will be helpful here. What fields (other than monitoring) do you think we should track?

(...)

Why is there bool sleep in both lunatik_class_t and lunatik_object_t. Is it safe to assume that sleep in lunatik_class_t is redundant?

No, it isn't. Object has precedence over class. For classes, sleep = true means that their objects are allowed to sleep, that is, their objects can either be used on contexts that might sleep or not. Thus, classes with sleep = true can have objects with both sleep = false and sleep = true. However, classes with sleep = false can only have objects with sleep = false. A good example is luadata and luanetfilter, the latter cannot sleep because it's called from softirqs; the former can be used on both contexts. Please, let me know if you need more clarification on this.

Lock type is unclear here (mutex vs spinlock), for instance, what happens if a wrong lock is taken?

How do you see the wrong lock being taken? It shouldn't, but perhaps we have a bug there.

I thought on having a bitfield carrying the object flags, including "shared" ("monitored") and "sleep". What do you think?

Yes, a flag will be helpful here. What fields (other than monitoring) do you think we should track?

I was thinking on having a RCU mode in the future where we lock only for writings, otherwise we use rcu_read_lock. Maybe some allocation flags. I didn't anticipate much on this, but I think it would be good to have some capabilities.

lunatik.h

lneto · 2026-01-24T12:06:44Z

lunatik.h

+			lua_pop(L, 1);
+			continue;
+		}
+		if (reg->name[0] == '_' && reg->name[1] == '_') {


I think strcmp would be clearer here, and even clearer to have a macro defined with it.. something like lunatik_ismetamethod, perhaps carrying the whole OR'ed cases.

lneto · 2026-01-25T13:44:59Z

Methods are now wrapped once in lunatik_newclass(). This reduces metatable churn and avoids per-access closure allocation in performance-sensitive paths.

Tested performance with the following script
local linux = require("linux")
local data = require("data")

local d = data.new(1024)
local iterations = 1000000

local start_ns = linux.time()
for i = 1, iterations do
    d:getbyte(0)
end
local end_ns = linux.time()
local total_ns = end_ns - start_ns

local ns_per_call = total_ns / iterations

print("Total ns: " .. total_ns)
print("Ns per call: " .. ns_per_call)
master: ~300 ns/call this branch: ~166 ns/call

can you also add to your tests that case of local d_getbyte = d.getbyte? so we can always compare the three cases? thanks!

lneto · 2026-01-28T11:39:44Z

lib/luacrypto_aead.c

 	.name = "crypto_aead",
 	.methods = luacrypto_aead_mt,
 	.release = luacrypto_aead_release,
 	.sleep = true,


we could add .monitor here (perhaps .shared is a better name).

yes, indeed. I have pushed a fix which checks closure creation on class level. We need to effectively test the modules having .shared = false also. Perhaps luarcu could be used for that.

lneto · 2026-01-28T11:42:52Z

lunatik.h

 	return hasindex;
 }

+static int lunatik_dispatch(lua_State *L)


you missed inline here.. however, I think this should be moved to lunatik_obj.c, we don't need to expose these internals to the bindings..

We don't need this dispatch, I think, I will push another fixup since we dont need to wrap anything in case class has .shared = false

lneto · 2026-01-28T11:45:38Z

lunatik.h

+	lunatik_object_t *obj = lunatik_checkobject(L, 1);
+	lua_CFunction fn = lua_tocfunction(L, lua_upvalueindex(1));
+
+	/* fast path, direct function call */


I don't think we need comments like this and the one bellow.. you code is quite clear in those points ;-).. let's use comments as exceptions, not the norm..

lneto · 2026-01-28T11:47:38Z

lunatik.h

+	const luaL_Reg *reg;
+	for (reg = class->methods; reg->name != NULL; reg++) {
+		lua_getfield(L, -1, reg->name);
+		if (lunatik_ismetamethod(reg)) {


we can avoid wrapping entirely if class is not monitored..

this could speedup even more the "fast path"

Yes, I realized this. Pushing another fixup to speed up the fast path.

Updated the description with latest results. Looks like we have some performance gains :D

lneto · 2026-01-28T11:55:04Z

lunatik_obj.c

 	lunatik_setobject(object, class, class->sleep);
 	lunatik_setclass(L, class);

+	object->monitored = true;


for setting this, we could change our C APIs to pass opt instead sleep, then we bundle all options we need as this argument.. for now, it can be only sleep and shared/monitored, then we can evolve.. does it make sense to you?

wrap class methods eagerly instead of via __index

8ae3e65

Methods are now wrapped once in lunatik_newclass(). This reduces metatable churn and avoids per-access closure allocation in performance-sensitive paths. Signed-off-by: Ashwani Kumar Kamal <ashwanikamal.im421@gmail.com>

sneaky-potato mentioned this pull request Jan 24, 2026

fix opaque error messages in lunatik_monitor #398

Open

lneto reviewed Jan 24, 2026

View reviewed changes

lunatik.h Outdated Show resolved Hide resolved

lneto reviewed Jan 24, 2026

View reviewed changes

lunatik.h Outdated Show resolved Hide resolved

lneto reviewed Jan 24, 2026

View reviewed changes

fixup! wrap class methods eagerly instead of via __index

e43c630

lneto reviewed Jan 28, 2026

View reviewed changes

fixup! wrap class methods eagerly instead of via __index

0401cec

wrap class methods eagerly instead of via __index #410

Are you sure you want to change the base?

wrap class methods eagerly instead of via __index #410

Conversation

sneaky-potato commented Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lneto commented Jan 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

sneaky-potato commented Jan 24, 2026 •

edited

Loading