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

Port JIT debug settings to the new config system #9001

Closed
wants to merge 2 commits into from

Conversation

OatmealDome
Copy link
Member

@OatmealDome OatmealDome commented Aug 3, 2020

Title. I think this is how you do a "depends-on" PR?

Depends on #8976.

JosJuice and others added 2 commits July 22, 2020 22:46
Other than the controller settings and JIT debug settings,
these are the only settings which were defined in Java code
but not defined in the new config system in C++. (There are
still a lot of settings that are defined in the new config
system but not yet saveable in the new config system, though.)
@Techjar
Copy link
Contributor

Techjar commented Aug 3, 2020

I'm not sure this is acceptable in its current form. Lookups in onion config are very slow (relatively speaking) due to string comparisons, and JIT functions are pretty hot during codegen, so this is likely going to have a noticeable impact on how long it takes to JIT a block.

@JosJuice
Copy link
Member

JosJuice commented Aug 3, 2020

Yep. That is why I didn't do the JIT debug settings in my PR, because I wasn't sure about what approach to use for caching the values.

@OatmealDome
Copy link
Member Author

OatmealDome commented Aug 3, 2020

Fair enough. I only really need part of this for my fork anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants