Skip to content

Conversation

@Physic69
Copy link
Contributor

No description provided.

Physic69 and others added 2 commits December 20, 2025 01:48
lunatik.h Outdated
#define LUNATIK_VERSION "Lunatik 3.7"

#define LUNATIK_CLASS_SLEEPABLE 0x01
#define LUNATIK_CLASS_NOSHARE 0x02
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do you handle allocations? we could have "single" mode objects that might or might not sleep on allocation, besides the fact they aren't shared. Perhaps, a better idea is to have two bools instead of flags, sleep and shared. What do you think?

Comment on lines +20 to +21
#define lunatik_object_issleepable(obj) ((obj)->sleep)
#define lunatik_class_issleepable(cls) ((cls)->sleep)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define lunatik_object_issleepable(obj) ((obj)->sleep)
#define lunatik_class_issleepable(cls) ((cls)->sleep)
#define lunatik_issleepable(o) ((o)->sleep)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, I wouldn't change this on the same PR.. it seems only cosmetic and not require to the main change your are adding.. let's keep the patches as self-contained as possible..

const luaL_Reg *methods;
void (*release)(void *);
bool sleep;
bool shared;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should invert this flag.. so we don't need to change all modules and just the places we really need (such as pointer)..

Suggested change
bool shared;
bool single;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually.. I wouldn't add this to class now, we don't need classes that are exclusively "single", right? I would just have it on object

Comment on lines +61 to +62
if (!object->shared)
luaL_error(L, "%s objects cannot be shared across runtimes", object->class->name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not enough, right? we probably need to add single flag to the API for creating new objects..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, avoiding creating mutex/spinlock and refcount unnecessarily..

Comment on lines 91 to 94
union {
struct mutex mutex;
spinlock_t spin;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should have a substruct allocated dynamically for kref and lock.. but let's worry about this after having the first version settled..

#define LUADATA_OPT_NONE 0x00
#define LUADATA_OPT_READONLY 0x01
#define LUADATA_OPT_FREE 0x02
#define LUADATA_OPT_SINGLE 0x04
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can just use the obj flag, right?

static int luadata_lnew(lua_State *L)
{
size_t size = (size_t)luaL_checkinteger(L, 1);
bool single = lua_toboolean(L, 2); /* false -> sharable */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool single = lua_toboolean(L, 2); /* false -> sharable */
bool single = lua_toboolean(L, 2);

#define lunatik_class_issleepable(cls) ((cls)->sleep)

#define lunatik_locker(o, mutex_op, spin_op) \
do { \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should check if it's single, right? it should lock in that case.. also, refcount shouldn't be applied if single

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants