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

async await 2 #148

Merged
merged 28 commits into from
Jun 17, 2021
Merged

async await 2 #148

merged 28 commits into from
Jun 17, 2021

Conversation

oberblastmeister
Copy link
Collaborator

No description provided.

@oberblastmeister oberblastmeister changed the title [WIP] async await 2 async await 2 Apr 25, 2021
@oberblastmeister
Copy link
Collaborator Author

@lewis6991 @RianFuro @mfussenegger breaking changes!

changes

remove async and await functions.

Async functions can be called normally provided that the root function is wrapped with a coroutine.

Improved speed

he async and await functions have been removed because they create closures and thunks every time which can cause significant slowdowns. Every async function would be a closure and calling them would creating a thunk by packing the arguments and then unpack when the thunk is called, which takes significant time. If you want to create a thunk yourself for you can using tools from plenary/fun.lua. This can be useful when you want to join the same function but with different arguments.

Vararg generation

We do a code generation for varargs of up to 16. This way we do not have to pack and unpack or insert a value into the table. This is necessary because you cannot append values to a vararg. See `plenary/vararg/rotate.lua. Rotating 5 arguments is benchmarked 2e7 times. Packing and unpacking takes a massive 13 seconds while the generated vararg functions only take 0.015 seconds.

Benefits

Benefits of removing async and await is that you can just call async functions like normal functions, provided the root function is wrapped with a coroutine. This feels much more idiomatic for lua and makes the implementation less colored. It also shows the power of coroutines, they can yield across almost any boundary. This means you can use functions that take regular functions and pass in an async function. For example, you can pass an async function into pcall, and the coroutine will yield through it. Another cool thing is that you can yield through iterators with the exact same syntax. For example:

-- now iterators just work like normal
for entry in readdir() do
end

-- instead of
local iter = await(readdir())
while true do
  local entry = await(iter())
  if entry == nil then break end
end

While other languages have their own syntax for awaiting iterators, we can just use the power of lua coroutines! Checkout out #150 for the real readdir iterator.

name changes

Because we are not working with futures anymore and we are just working with functions, some names have changed.

  • run_all, await_all -> join
  • channel condvar etc is moved to control.lua

Copy link
Member

@tjdevries tjdevries left a comment

Choose a reason for hiding this comment

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

Main thing we should decide is how to transition from current async_lib to new async_lib. I don't want two async libs in the same repo :)

What are your thoughts for that?

lua/plenary/async_lib2/util.lua Outdated Show resolved Hide resolved
lua/plenary/vararg/init.lua Outdated Show resolved Hide resolved
lua/plenary/vararg/rotate.lua Outdated Show resolved Hide resolved
@oberblastmeister
Copy link
Collaborator Author

The reason I have two libraries is because if I were to delete the old async library it would be a pain to develop because I use both neogit and gitsigns which both depend on it. My plan was to keep both libraries for a while to allow plugins to migrate without errors and then delete the old library.

@tjdevries
Copy link
Member

Yeah, I understand we don't want to break their code right now, but since the two pieces of code are not compatible, there is no reason to model async_lib2 to have the exact same words as async_lib -- we should pick the best language for async_lib2 as it will be the standard going forward.

Additionally, we should think of some way to transition these to a different version, for example, require('plenary.async') instead of async_lib2 or some other name we don't want in the future. I'd have to think about that some more before I had any good ideas. Open to suggestions there.

@oberblastmeister
Copy link
Collaborator Author

oberblastmeister commented May 26, 2021

Yes I understand. I have removed all the compatibility. Are you saying you want me to change the name to async instead of async_lib2?

step()
end

local add_leaf_function
Copy link
Member

Choose a reason for hiding this comment

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

Only thing I thought here was you might run into problems if people do something like require('plenary.async') vs. require('plenary/async') -- this will give you two different local tables. You may want to do the trick we do other places of using a global and local leaf_table = _PlenaryLeafTable etc.


sent = true

local args = {...}
Copy link
Member

Choose a reason for hiding this comment

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

we can probably do this without storing args into a table, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I made it so it won't store into table for zero or one args, other than that I think it has to store into a table

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.

3 participants