Synchronization updates: final and volatile fields, immutable struct #284

pitr-ch · 2015-04-28T13:29:56Z

Add ns_initialize for ivars setting inside synchronize block
cleans up the Synchronization::Object space
add ensure_ivar_visibility! for final fields
add volatile attributes

- no to pollute the object's space - it can be easily added as noted in documentation

pitr-ch · 2015-04-28T21:12:16Z

After applying the new capabilities of Synchronization::Object to Edge::Future on the obvious places the performance improved. It now has better or same performance as old implementations.

Calculating -------------------------------------
           value-old    19.844k i/100ms
           value-new    21.473k i/100ms
-------------------------------------------------
           value-old    300.837k (±13.9%) i/s -      2.937M
           value-new    295.720k (±18.5%) i/s -      2.813M

Comparison:
           value-old:   300836.7 i/s
           value-new:   295720.0 i/s - 1.02x slower

Calculating -------------------------------------
           graph-old    59.000  i/100ms
           graph-new   231.000  i/100ms
-------------------------------------------------
           graph-old    777.004  (±32.9%) i/s -      6.431k
           graph-new      2.211k (±25.2%) i/s -     20.328k

Comparison:
           graph-new:     2211.0 i/s
           graph-old:      777.0 i/s - 2.85x slower

Calculating -------------------------------------
       immediate-old   924.000  i/100ms
       immediate-new   969.000  i/100ms
-------------------------------------------------
       immediate-old      9.124k (±23.4%) i/s -     84.084k
       immediate-new      9.134k (±24.7%) i/s -     82.365k

Comparison:
       immediate-new:     9133.6 i/s
       immediate-old:     9123.7 i/s - 1.00x slower

Calculating -------------------------------------
            then-old   402.000  i/100ms
            then-new   410.000  i/100ms
-------------------------------------------------
            then-old      4.306k (±28.2%) i/s -     36.984k
            then-new      4.506k (±27.1%) i/s -     40.590k

Comparison:
            then-new:     4505.6 i/s
            then-old:     4306.2 i/s - 1.05x slower

Before with only synchronize it was:

Calculating -------------------------------------
           value-old    17.623k i/100ms
           value-new    11.537k i/100ms
-------------------------------------------------
           value-old    293.436k (±15.9%) i/s -      2.837M
           value-new    254.683k (±13.1%) i/s -      2.504M

Comparison:
           value-old:   293436.4 i/s
           value-new:   254682.8 i/s - 1.15x slower

Calculating -------------------------------------
           graph-old    73.000  i/100ms
           graph-new   237.000  i/100ms
-------------------------------------------------
           graph-old    863.588  (±26.5%) i/s -      7.592k
           graph-new      2.502k (±18.0%) i/s -     23.700k

Comparison:
           graph-new:     2501.6 i/s
           graph-old:      863.6 i/s - 2.90x slower

Calculating -------------------------------------
       immediate-old   956.000  i/100ms
       immediate-new   678.000  i/100ms
-------------------------------------------------
       immediate-old      9.920k (±21.0%) i/s -     92.732k
       immediate-new      6.752k (±13.1%) i/s -     65.766k

Comparison:
       immediate-old:     9919.6 i/s
       immediate-new:     6752.0 i/s - 1.47x slower

Calculating -------------------------------------
            then-old   518.000  i/100ms
            then-new   519.000  i/100ms
-------------------------------------------------
            then-old      5.312k (±23.5%) i/s -     48.174k
            then-new      4.431k (±19.5%) i/s -     41.520k

Comparison:
            then-old:     5312.2 i/s
            then-new:     4431.2 i/s - 1.20x slower

jdantonio · 2015-04-29T00:48:27Z

lib/concurrent/ivar.rb

@@ -66,6 +66,8 @@ def initialize(value = NO_VALUE, opts = {})
      set(value) unless value == NO_VALUE
    end

+    public :synchronize


Why is this public and not protected?

Good point, thanks. There should be protected.

can be made public if required in descendant

pitr-ch · 2015-04-29T19:14:32Z

Documentation added, see https://github.com/ruby-concurrency/concurrent-ruby/blob/synchronization/doc/synchronization.md

jdantonio · 2015-04-29T22:31:19Z

lib/concurrent/synchronization/immutable_struct.rb

+        Class.new(self) do
+          attr_reader(*names)
+
+          class_eval <<-RUBY, __FILE__, __LINE__ + 1


What is the advantage of class_eval over define_method?

Only performance improvements in some cases, otherwise define_method is cleaner. I wanted to avoid storing instance variables using reflection (which is optimized only on Truffle AFAIK).

One eval removed in 3dbc08f.

jdantonio · 2015-04-30T01:45:46Z

👍

…ations

jdantonio · 2015-04-30T13:02:40Z

👍 I'm 👌 with merging this. I'll rebase and continue working on #271, #283, and #280 afterwards.

pitr-ch · 2015-04-30T13:35:13Z

Unfortunately there is an almost consistently failing test in rbx, I'll try to fix it first and then I'll merge.

pitr-ch · 2015-05-02T09:30:15Z

The failing test is not using Synchronized::Object and I cannot get it reproduced :/ If It succeeds at least once on travis I'll merge this branch and add it to intermittently failing tests.

jdantonio · 2015-05-02T11:33:58Z

I would prefer to merge this branch then work on the failing test afterwards:

This code needs to be in master for is to proceed with our other work
As noted, the failing test appears to be unrelated to the changes in this PR
I spent over an hour the other day trying to get Rbx to install on an Ubuntu laptop and wasn't successful, so fixing this test may be very difficult

* master: (5 commits) Deprecate timeout ...

… locals

pitr-ch · 2015-05-02T13:18:18Z

Merging since travis gave a different intermittent error.

Synchronization updates: final and volatile fields, immutable struct

jdantonio · 2015-05-02T13:18:48Z

👍

jdantonio · 2015-05-02T13:20:18Z

I was able to install Rbx on my MacBook Pro and I've been unable to reproduce the failed test. I think it's safe to call this an intermittent failure and add it to the list.

pitr-ch · 2015-05-02T13:25:52Z

@jdantonio added.

jdantonio added the in progress label Apr 28, 2015

pitr-ch added the enhancement Adding features, adding tests, improving documentation. label Apr 28, 2015

pitr-ch added this to the 0.9.0 Release milestone Apr 28, 2015

pitr-ch force-pushed the synchronization branch 2 times, most recently from 13f4088 to 2166f0f Compare April 28, 2015 16:18

pitr-ch added 2 commits April 28, 2015 21:10

Add ns_initialize which is always called inside synchronize block

6dbfa92

Remove wait, signal, broadcast in favor of ns_ counterparts

2a182b4

- no to pollute the object's space - it can be easily added as noted in documentation

pitr-ch force-pushed the synchronization branch from 2166f0f to 686d661 Compare April 28, 2015 19:10

pitr-ch changed the title ~~Synchronization updates~~ Synchronization updates, final and volatile fields Apr 28, 2015

jdantonio reviewed Apr 29, 2015
View reviewed changes

pitr-ch added 2 commits April 29, 2015 10:38

Make synchronize private by default

505d6dd

can be made public if required in descendant

Add ensure_ivar_visibility! and volatile attributes

08c2423

pitr-ch force-pushed the synchronization branch from 7b53868 to 4ca5220 Compare April 29, 2015 08:38

pitr-ch changed the title ~~Synchronization updates, final and volatile fields~~ Synchronization updates: final and volatile fields; immutable struct Apr 29, 2015

pitr-ch changed the title ~~Synchronization updates: final and volatile fields; immutable struct~~ Synchronization updates: final and volatile fields, immutable struct Apr 29, 2015

pitr-ch force-pushed the synchronization branch 2 times, most recently from 9177c18 to 83372c6 Compare April 29, 2015 09:44

This was referenced Apr 29, 2015

Synchronization ExecutorService #283

Merged

Move Exchanger and Lazy Register to Edge Gem #279

Closed

pitr-ch force-pushed the synchronization branch from baf8c5d to 3a71338 Compare April 29, 2015 21:16

jdantonio reviewed Apr 29, 2015
View reviewed changes

Add ImmutableStruct implementation

41b7adf

pitr-ch added 6 commits April 30, 2015 09:05

Apply new Synchronization::Object capabilities to Edge::Future

5c4425f

ImmutableStruct Improvements

1ba4f88

Test stabilization

47cde7b

Synchronization module documentation

310b7df

Move ensure_ivar_visibility! ruby method the the Java extension on JRuby

2907026

Move Object definition to its own file

b94357b

pitr-ch force-pushed the synchronization branch from fe0860e to 6f68140 Compare April 30, 2015 07:05

pitr-ch added 3 commits April 30, 2015 13:09

Remove unnecessary definition of members methods by eval

8c92b51

Extract ruby_version comparison

32162e0

Making Event and Future fields volatile to remove few more synchroniz…

3dc0e87

…ations

pitr-ch force-pushed the synchronization branch from 6f68140 to 3dc0e87 Compare April 30, 2015 11:09

pitr-ch mentioned this pull request May 2, 2015

AtomicMarkableReference: Add pure Ruby implementation #281

Merged

pitr-ch added 3 commits May 2, 2015 14:54

Merge branch 'master' into synchronization

df217b2

* master: (5 commits) Deprecate timeout ...

Shorten the lock and condition ivar names

cab6395

Ensure task arguments are supplied by block arguments not by captured…

67a82f7

… locals

pitr-ch pushed a commit that referenced this pull request May 2, 2015

Merge pull request #284 from ruby-concurrency/synchronization

37edfe3

Synchronization updates: final and volatile fields, immutable struct

pitr-ch merged commit 37edfe3 into master May 2, 2015

jdantonio removed the in progress label May 2, 2015

jdantonio deleted the synchronization branch May 7, 2015 18:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Synchronization updates: final and volatile fields, immutable struct #284

Synchronization updates: final and volatile fields, immutable struct #284

pitr-ch commented Apr 28, 2015

pitr-ch commented Apr 28, 2015

jdantonio Apr 29, 2015

pitr-ch Apr 29, 2015

pitr-ch commented Apr 29, 2015

jdantonio Apr 29, 2015

pitr-ch Apr 30, 2015

pitr-ch Apr 30, 2015

jdantonio commented Apr 30, 2015

jdantonio commented Apr 30, 2015

pitr-ch commented Apr 30, 2015

pitr-ch commented May 2, 2015

jdantonio commented May 2, 2015

pitr-ch commented May 2, 2015

jdantonio commented May 2, 2015

jdantonio commented May 2, 2015

pitr-ch commented May 2, 2015

Synchronization updates: final and volatile fields, immutable struct #284

Synchronization updates: final and volatile fields, immutable struct #284

Conversation

pitr-ch commented Apr 28, 2015

pitr-ch commented Apr 28, 2015

jdantonio Apr 29, 2015

Choose a reason for hiding this comment

pitr-ch Apr 29, 2015

Choose a reason for hiding this comment

pitr-ch commented Apr 29, 2015

jdantonio Apr 29, 2015

Choose a reason for hiding this comment

pitr-ch Apr 30, 2015

Choose a reason for hiding this comment

pitr-ch Apr 30, 2015

Choose a reason for hiding this comment

jdantonio commented Apr 30, 2015

jdantonio commented Apr 30, 2015

pitr-ch commented Apr 30, 2015

pitr-ch commented May 2, 2015

jdantonio commented May 2, 2015

pitr-ch commented May 2, 2015

jdantonio commented May 2, 2015

jdantonio commented May 2, 2015

pitr-ch commented May 2, 2015