Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to Heap-allocated Call Stack and Performance Improvements #606

Merged
merged 3 commits into from
Jun 24, 2024

Conversation

singul4ri7y
Copy link
Contributor

@singul4ri7y singul4ri7y commented Jun 19, 2024

Change log:

  1. Migrated to Heap-allocated stack where the allocation is completely handled by Pallene (using malloc and free, because we cannot use regular userdata here).
  2. Added pallene-debug script which will allow one to debug Pallene modules without making any changes to the Lua script.
  3. Added finalizers and other optimizations to yield maximum performance which fixes a critical bug.

Resolves #604

@singul4ri7y singul4ri7y self-assigned this Jun 19, 2024
src/bin/pallene-debug Outdated Show resolved Hide resolved
src/bin/pallene-debug Outdated Show resolved Hide resolved
src/bin/pallene-debug Outdated Show resolved Hide resolved
src/bin/pallene-debug Outdated Show resolved Hide resolved
Copy link
Member

@hugomg hugomg left a comment

Choose a reason for hiding this comment

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

I have some bigger questions about the pallne-debug.

The C bits look good at a first glance but I still need to take a closer look at them.

end

-- Moment of truth.
xpcall(wrapper, pallene_tracer_debug_traceback)
Copy link
Member

Choose a reason for hiding this comment

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

I think we do not need a wrrapper.

xpcall(fn, traceback, table.unpack(opts.args))


-- Run the script initially so that we can figure out whether
-- we have Pallene modules that is compiled with `--use-traceback` flag.
local _, _ = pcall(fn)
Copy link
Member

Choose a reason for hiding this comment

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

Running the script twice sounds sketchy. It can have other side effects other than printing to screen, and it will take twice as long.

I suppose it would make more sense for the pallene-debug script to supply the traceback function, instead of getting it from a global variable in the loaded script.

Maybe what we should do is create a standalone C module that exports the Pallene traceback and install it together with pallenec. Then, in this script we can require the traceback function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Say the Lua script didn't use any Pallene module with traceback. What then? If we supply our own traceback function it might segfault because there is no stack to begin with. That would be risky. We need to make sure somehow that a Pallene module with traceback exists.

Copy link
Member

Choose a reason for hiding this comment

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

Could the traceback function check the Lua registry to see if the Pallene stack exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could.

Maybe it's a good idea to detach the traceback fn once and for all. Because we are using it explicitly anyway. But when calling with xpcall, can the traceback fn return anything? Maybe we can introduce another fn for that?

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand the "when calling with xpcall" question.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The traceback fn should return anything to denote there is no stack to work on. This way we would be able to warn the users to use the --use-traceback flag. It would be better than not printing any traceback when there is no Pallene stack at all.

My question was if we pass our traceback to xpcall as second argument can we fetch what it returned? Or we have to introduce another fn for it?

Copy link
Contributor Author

@singul4ri7y singul4ri7y Jun 21, 2024

Choose a reason for hiding this comment

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

On the contrary, I have a grosshack (sorta) prepared. Though we have to run Lua scripts twice, it won't be slow. Also it shouldn't have side effects.

Creating a separate module, exporting traceback fn is way too hassle for a single debug script in my opinion.

@hugomg hugomg merged commit 3e2846a into master Jun 24, 2024
2 checks passed
@hugomg hugomg deleted the stack-trace-improvements branch June 24, 2024 06:00
@singul4ri7y singul4ri7y restored the stack-trace-improvements branch August 7, 2024 13:44
@singul4ri7y singul4ri7y deleted the stack-trace-improvements branch August 7, 2024 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stacktrace: Reset Pallene stack when done printing tracebacks to avoid stack corruption after pcall
2 participants