Skip to content

Conversation

@depombo
Copy link
Member

@depombo depombo commented Dec 15, 2020

No description provided.

@dfellis
Copy link
Member

dfellis commented Dec 15, 2020

This doesn't actually fix the real issue of #354 which is that we're still creating a tokio task per array element, which explodes the AVM with an OOM on large arrays unnecessarily (also because we duplicate the working memory for each task right now, so we get array size to the array size amount of memory consumption)

documentation = "https://docs.alan-lang.org"
repository = "https://github.com/alantech/alan"
version = "0.1.22"
version = "0.1.48"
Copy link
Member

Choose a reason for hiding this comment

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

Is this the Rust version? Or why did you change it?

Copy link
Member Author

Choose a reason for hiding this comment

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

this is the Rust version

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

It appears you can't specify the Rust compiler version in Cargo? emk/heroku-buildpack-rust#11 That explains why we've had such difficulties with the submitted projects compiling on our laptops.

Copy link
Member

Choose a reason for hiding this comment

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

Tracing through several years of github issues leads me to: rust-lang/rustup#1172

We apparently need to add a rust-toolchain file that contains, as far as I can tell, only the version of rust that we want to use?

version = "0.1.22"
version = "0.1.48"
authors = ["Luis de Pombo <[email protected]>", "David Ellis <[email protected]>"]
edition = "2018"
Copy link
Member

Choose a reason for hiding this comment

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

I assumed this was the field you need to change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Initially, I thought so too

@dfellis
Copy link
Member

dfellis commented Dec 15, 2020

It looks like this upgrade also broke the HTTP client and server code :/

@depombo
Copy link
Member Author

depombo commented Dec 15, 2020

This doesn't actually fix the real issue of #354 which is that we're still creating a tokio task per array element, which explodes the AVM with an OOM on large arrays unnecessarily (also because we duplicate the working memory for each task right now, so we get array size to the array size amount of memory consumption)

I see. To some extent I did not understand the issue correctly. It should not spin one thread per array element, but also it should not spin on task per array element. Since tokio uses a work stealing mechanism this addresses the former, but not the latter.

@depombo
Copy link
Member Author

depombo commented Dec 15, 2020

It looks like this upgrade also broke the HTTP client and server code :/

Yeah, I think this is a bug between enable_all not actually turning on enable_io

@dfellis
Copy link
Member

dfellis commented Dec 15, 2020

It looks like this upgrade also broke the HTTP client and server code :/

Yeah, I think this is a bug between enable_all not actually turning on enable_io

...that's crazy! What else would you mean by "enable_all"?

@depombo
Copy link
Member Author

depombo commented Dec 15, 2020

It looks like this upgrade also broke the HTTP client and server code :/

Yeah, I think this is a bug between enable_all not actually turning on enable_io

...that's crazy! What else would you mean by "enable_all"?

So it turns out that it was silently failing because the latest version of hyper and reqwst are still using tokio 0.2. hyper's master branch already made the change, but until their next release we can't upgrade. I'm going to abandon and make a new PR where I fix the actual issue of spinning up a tokio task per array element and set max threads without upgrading tokio

@depombo depombo closed this Dec 15, 2020
@dfellis dfellis deleted the cap-threads branch April 9, 2024 03:06
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