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

Always search full hierarchy for highest definition of a cvar. #5175

Merged
merged 1 commit into from Sep 19, 2018

Conversation

Projects
None yet
3 participants
@headius
Copy link
Member

headius commented May 16, 2018

This makes the feature basically useless for anything requiring
performance, since it always has to search the entire hierarchy
for a late-assigned value further up.

Additional work using our weak subclass hierarchy might be able
to make reads slightly cheaper (giving up at first successfuly
read) by walking down the hierarchy on every write and killing
other vars, but the concurrency aspects of that make my head spin.

Fixes #1554.

Always search full hierarchy for highest definition of a cvar.
This makes the feature basically useless for anything requiring
performance, since it always has to search the entire hierarchy
for a late-assigned value further up.

Additional work using our weak subclass hierarchy might be able
to make reads slightly cheaper (giving up at first successfuly
read) by walking down the hierarchy on every write and killing
other vars, but the concurrency aspects of that make my head spin.

Fixes #1554.

@headius headius added this to the JRuby 9.2.0.0 milestone May 16, 2018

@enebo enebo modified the milestones: JRuby 9.2.0.0, JRuby 9.2.1.0 May 24, 2018

@kares

This comment has been minimized.

Copy link
Member

kares commented Jun 1, 2018

... seems to fail (regress) some MRI tests? or maybe it just needs a rebase

@enebo enebo merged commit a0506c6 into jruby:master Sep 19, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@enebo

This comment has been minimized.

Copy link
Member

enebo commented Sep 19, 2018

I applied this locally and those MRI tests are fine. Probably related to my symbol love work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.