Skip to content

revert volatile mod from values[] RubyArray store?#5348

Merged
headius merged 1 commit intomasterfrom
non-volatile-array-values
Oct 11, 2018
Merged

revert volatile mod from values[] RubyArray store?#5348
headius merged 1 commit intomasterfrom
non-volatile-array-values

Conversation

@kares
Copy link
Member

@kares kares commented Oct 5, 2018

maybe I am missing smt but from reviewing parts doesn't bring much value

from the commit desc that introduced it ... seems to be due nil fills after assignment, makes no sense?

RubyArray isn't thread-safe to start with and this might help improper concurrent usage

... but only values has been volatile and not begin or length fields - so detection only works with internal array resizing and even than the other thread's view might not be correct, due cached begin field

first of all RubyArray isn't thread-safe to start with

volatile modifier was introduced at bdf50fd
new array nil-fills are now done before field assignment
@headius
Copy link
Member

headius commented Oct 11, 2018

I approve. RubyArray may deserve some proper volatility checks in the future but this partial change was not really sufficient to make sure the new state of the array is properly visible. It also adds volatile write overhead to every (already not thread-safe) update of the RubyArray values store.

@headius headius added this to the JRuby 9.2.1.0 milestone Oct 11, 2018
@headius headius merged commit 241f698 into master Oct 11, 2018
@headius headius deleted the non-volatile-array-values branch October 11, 2018 19:05
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