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

refactor(luasnip): minor code improvements and cleanup #1060

Closed
wants to merge 1 commit into from

Conversation

soifou
Copy link
Contributor

@soifou soifou commented Jan 23, 2025

- Cache module to avoid repeated require calls

  • Use more efficient table insertion methods
  • Remove dups in cached snippets introduced in commit #712af9f
  • Improve code readability

@Saghen
Copy link
Owner

Saghen commented Jan 23, 2025

The require('luasnip') calls should be free after the first call: https://www.lua.org/pil/8.1.html. Do you have before and after numbers for the table insertion?

- Improve code readability
- Use more efficient table insertion methods
- Remove dups in cached snippets introduced in commit #712af9f
@soifou soifou changed the title refactor(luasnip): optimize source and improve performance refactor(luasnip): minor code improvements and cleanup Jan 23, 2025
@soifou
Copy link
Contributor Author

soifou commented Jan 23, 2025

That description was "click-baity" I owe you that, I updated the message to something more in phase. I know the subsequent require calls take no costs, that was mostly for readability.

The removal of dups + table lookup when inserting items was the motivation for using a "perf" word but I was more concerned by the dups.

As for the numbers, this is mostly negligible, I did a quick n' cheap bench on my machine with 20 iterations on 2 runs, master vs this branch (added extra 10ms to get something readable).

local time_elapsed = os.clock()
-- source:get_completion()
vim.uv.sleep(10)                                              
time_elapsed = os.clock()-time_elapsed                        
vim.print(("Completed in %.5f seconds"):format(time_elapsed))

Results

# Master (run 1)
Completed in 0.00133 seconds
Completed in 0.00112 seconds
Completed in 0.00166 seconds
Completed in 0.00063 seconds
Completed in 0.00053 seconds
Completed in 0.00034 seconds
Completed in 0.00056 seconds
Completed in 0.00151 seconds
Completed in 0.00055 seconds
Completed in 0.00049 seconds
Completed in 0.00063 seconds
Completed in 0.00062 seconds
Completed in 0.00064 seconds
Completed in 0.00043 seconds
Completed in 0.00086 seconds
Completed in 0.00064 seconds
Completed in 0.00038 seconds
Completed in 0.00041 seconds
Completed in 0.00048 seconds
Completed in 0.00055 seconds
# Run 2
Completed in 0.00102 seconds
Completed in 0.00055 seconds
Completed in 0.00133 seconds
Completed in 0.00098 seconds
Completed in 0.00050 seconds
Completed in 0.00051 seconds
Completed in 0.00058 seconds
Completed in 0.00196 seconds
Completed in 0.00064 seconds
Completed in 0.00070 seconds
Completed in 0.00056 seconds
Completed in 0.00072 seconds
Completed in 0.00071 seconds
Completed in 0.00090 seconds
Completed in 0.00074 seconds
Completed in 0.00068 seconds
Completed in 0.00159 seconds
Completed in 0.00049 seconds
Completed in 0.00051 seconds
Completed in 0.00266 seconds

# This branch (Run 1)
Completed in 0.00132 seconds
Completed in 0.00078 seconds
Completed in 0.00074 seconds
Completed in 0.00041 seconds
Completed in 0.00049 seconds
Completed in 0.00109 seconds
Completed in 0.00070 seconds
Completed in 0.00063 seconds
Completed in 0.00039 seconds
Completed in 0.00221 seconds
Completed in 0.00050 seconds
Completed in 0.00050 seconds
Completed in 0.00061 seconds
Completed in 0.00101 seconds
Completed in 0.00033 seconds
Completed in 0.00064 seconds
Completed in 0.00027 seconds
Completed in 0.00034 seconds
Completed in 0.00075 seconds
Completed in 0.00068 seconds
# Run 2
Completed in 0.00118 seconds
Completed in 0.00055 seconds
Completed in 0.00089 seconds
Completed in 0.00040 seconds
Completed in 0.00036 seconds
Completed in 0.00100 seconds
Completed in 0.00081 seconds
Completed in 0.00043 seconds
Completed in 0.00042 seconds
Completed in 0.00041 seconds
Completed in 0.00049 seconds
Completed in 0.00076 seconds
Completed in 0.00044 seconds
Completed in 0.00048 seconds
Completed in 0.00054 seconds
Completed in 0.00046 seconds
Completed in 0.00038 seconds
Completed in 0.00045 seconds
Completed in 0.00082 seconds
Completed in 0.00045 seconds

@soifou
Copy link
Contributor Author

soifou commented Jan 26, 2025

Closing this PR since the duplicate items has been resolved in HEAD and the remaining changes debatable.

@soifou soifou closed this Jan 26, 2025
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