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

feat: Implement buffer profiler #22

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

connorgmeehan
Copy link

@connorgmeehan connorgmeehan commented Nov 11, 2022

Screen Shot 2022-11-11 at 7 03 30 pm

Adds a profiler to Luapad by adding the comment

-- PROFILE [iteration_count]

Just a draft PR because I would like some feedback. Any thoughts on how I can asyncronously update the virtual text? I'm thinking it shows the profiling percentage and the current average time i.e.
..1% -- 0.43354353ms avg (1 iteration)
.50% -- 0.43354353ms avg (5 iterations)
100% -- 0.43354353ms avg (10 iterations)

Also is this a suitable implementation for adding profiling in general? Should the check for -- PROFILE be a bit more structured i.e. something like -- #luapad:profile [var1] [var2] [var3] ...

Edit: "Benchmark" is probably a better term for this. Updated to use #luapad:benchmark [iter_count]

@connorgmeehan
Copy link
Author

connorgmeehan commented Nov 12, 2022

Screen Shot 2022-11-12 at 4 49 41 pm

I made the changes internally and the consistency is pretty great, even good enough for micro benchmarking. Seems like using vim.defer_fn works fine for clearing the CPU cache. I'll clean these changes up and update the PR later this week.
Do you have any preferences for for a highlight group to use for the message?

One issue is that neovim seems to throttle the setting of highlighted text forcing the user to continuously highlight the line over and over to update the virtual text. Do you know of a workaround? Maybe fidget.nvim has solved this.

Unrelated: I really like luapad as a sketchpad and would like to be able to save and maintain luapad files. What is the best way of doing this? Is this something that could be included in Luapad (I can PR) or is this something that I should implement on top of luapad? I.e. saving to ~/.local/share/nvim/luapad-sketches/**.lua, automatically binding luapad to buffers of files within this folder, possibly making a telescope searcher for previous sketches and opening them in a new window. It's such a powerful tool for prototyping and I keep wanting to refer back to stuff I've previously done.

@rafcamlet
Copy link
Owner

Whoa... That's sick idea! And works really great. 👍 I love it! ❤️

Do you have any preferences for for a highlight group to use for the message?

Nope. IncSearch is as good as any other. 👍

One issue is that neovim seems to throttle the setting of highlighted text forcing the user to continuously highlight the line over and over to update the virtual text. Do you know of a workaround?

Honestly, I'm not sure if there is a way to handle this for virtual text. For normal text, position-based syntax highlighting may do the trick, but it doesn't work with virtual text. I'll do some research. Maybe I'll find something. Do you think it has a significant impact on performance? Probably not, but faster is always better. :)

(...) and would like to be able to save and maintain luapad files.

I always wanted to prevent this behaviour because Luapad command was always intended for use and forget usage, and the "manual initialization" is more for working with existing or persisted files. But this is not the first time when I heard about this. Clearly, that's the will of the people! :D I understand that's the third option: some kind of sketches history.

Your proposition sounds very reasonable. About the path, I would stay with just luapad instead of luapad-sketches or at least luapad/sketches. Also, building a path based on the stdpath('data') function is a good practise. It should be pointing to the same directory. I like the idea of integration with Telescope, but first, it should be accessible via vim.ui.select().

Btw. I noticed that there are problems with benchmarking and displaying the preview window. It seems that user input is blocked when luapad is about to shows preview and benchmarking is not complete. Could you check if you have the same problem? If there is no way to fix that, I think we should just disable preview during benchmarking. Completely or only until benchmarking isn't finished. I'm wonder if it would require some changes in autocomands.

Great work!

local percentage = ("%s"):format(math.floor((cur_iteration / max_iterations) * 100))
local percentage_str = string.rep(" ", 3 - #percentage) .. percentage .. "%"
local avg_time = total_time / cur_iteration
handle_result(("BENCHMARK: %s ms (avg, %s of %s iterations)"):format(avg_time, percentage_str, max_iterations))
Copy link
Owner

Choose a reason for hiding this comment

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

The output is a little jumpy, so I would use fixed precision. Something like:

handle_result(("BENCHMARK: %.4f ms (avg, %s of %s iterations)"):format(tonumber(avg_time), percentage_str, max_iterations))

Feel free to change precision as you like.

@@ -102,6 +105,11 @@ function Evaluator:eval()
self.output = {}

local code = vim.api.nvim_buf_get_lines(self.buf, 0, -1, {})
local benchmark_line, benchmark_line_num = table_find(code, function (line)
return line:match("-- #luapad:benchmark") ~= nil
Copy link
Owner

Choose a reason for hiding this comment

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

return line:match("^ *%-%- *#luapad:benchmark")

The - char, has actually special meaning in lua pattern matching. Yeah, I know, tricky! It's a non-greedy version of *.

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.

2 participants