Fix repo location of rubyspec #508

mbj · 2015-12-19T22:18:06Z

No description provided.

* Fix bug exposed by new rubyspec location * Fix excludes on corpus to actually *do stuff*.

mbj · 2015-12-20T04:11:34Z

@dkubb Ready for review.

backus · 2015-12-21T00:48:19Z

spec/support/corpus.rb

+ # @return [Array<Pathname>]
+ def effective_ruby_paths
+ paths = Pathname
+ .glob(repo_path.join('**/*.rb'))


Think this glob string could be hoisted up into a constant?

Yes. Just like the other ones.

backus · 2015-12-21T00:58:45Z

@mbj I finished a pass over this PR

dkubb · 2015-12-21T01:09:54Z

spec/integrations.yml

@@ -1,14 +1,14 @@
 ---
 - name: rubyspec
 namespace: Rubyspec
- repo_uri: 'https://github.com/rubyspec/rubyspec.git'
+ repo_uri: 'https://github.com/ruby/rubyspec.git'


Has development on the rubyspec/rubyspec repo stopped while the ruby/rubyspec one continued?

The repo disappeared. Yes. And I found ruby/rubyspec to be the successor.

mbj · 2015-12-21T01:29:23Z

@dkubb Ready for the next pass.

backus · 2015-12-21T01:32:32Z

spec/support/corpus.rb

+ paths = Pathname
+ .glob(repo_path.join(RUBY_FILE_PATTERN))
+ .sort_by(&:size)
+ .reverse


I know this was in the previous code but I'm curious, why do you sort and reverse here?

In the following explanation I'll refer to the files in the corpus as item.

On large parallel, blocking (as in all parallel computations have to finish before we can move on) computations we ideally want to have the work most evenly distributed between the concurrent threads and they should all terminate the same moment.

So in the worst case one thread out of N is just, before all others finish their last item starting to process the item with the longest computation time. Adding significantly to the total runtime of the parallel computation in the most inefficient way.

We can avoid this worst case scenario (and also improve the average case!) via making sure that only small items are left when the queue drains, via starting to produce the heavy items early.

Now as part of processing an item in this specific computation we have the problem that we cannot easily estimate the weight of an item. A big file could also be made out of comments that cause no mutations.

But it turns out that using the file size as a cheap to check property is a good approximation to use this optimization.

You can try this for yourself and remove the optimization noticing that we slice some seconds of the overall runtime with this simple thing.

Very interesting. Thanks for explaining

dkubb · 2015-12-28T21:31:18Z

@mbj I will be ready to merge this once https://github.com/mbj/mutant/pull/508/files#r48119998 is done, and the build is green.

mbj · 2015-12-29T09:45:34Z

@mbj I will be ready to merge this once https://github.com/mbj/mutant/pull/508/files#r48119998 is done, and the build is green.

I hope I get to it today, all under the "on vacation" indeterminism.

dkubb · 2015-12-29T19:03:10Z

I hope I get to it today, all under the "on vacation" indeterminism.

Of course, no pressure.

mbj · 2015-12-30T18:42:08Z

@dkubb Ready for the next pass.

backus · 2015-12-30T19:48:22Z

spec/support/corpus.rb

@@ -10,10 +10,18 @@ module MutantSpec
 #
 # rubocop:disable MethodLength
 module Corpus
+ TMP = ROOT.join('tmp').freeze
+ EXCLUDE_GLOB_FORMAT = '{%s}'.freeze
+ RUBY_GLOB_PATTERN = '**/*.rb'


Can we freeze this?

mbj · 2015-12-30T20:28:57Z

@dkubb Ready for the next pass.

backus · 2015-12-30T21:45:28Z

You can also remove core/regexp/shared/new.rb from the excludes in the rubyspec integration since that file was deleted in ruby/spec@9614f25

dkubb · 2015-12-31T07:33:46Z

spec/support/corpus.rb

+ # @return [Array<Pathname>]
+ def excluded_paths
+ Pathname.glob(repo_path.join(EXCLUDE_GLOB_FORMAT % exclude.join(',')))
+ end

 # Number of parallel processes to use
 #
 # @return [Fixnum]
 def parallel_processes


I realize this isn't changed in this diff, but I wondered why this needs to be an instance method?

Its a helper method, and I do not want to make it part of the public API of the corpus class just to avoid having it as a helper method.

No Public API > "Bad" private API.

mbj · 2015-12-31T13:40:38Z

You can also remove core/regexp/shared/new.rb from the excludes in the rubyspec integration since that file was deleted in ruby/spec@9614f25

I wounder if the code should raise when an exclude does NOT trigger. But will delay that for the next iteration.

mbj · 2015-12-31T13:46:59Z

@backus Redundant exclude removed.

@dkubb Ready for the next pass.

Fix repo location of rubyspec

Fix repo location of rubyspec

fe1deba

* Fix bug exposed by new rubyspec location * Fix excludes on corpus to actually *do stuff*.

mbj force-pushed the fix/rubyspec-location branch 3 times, most recently from d298e92 to 7b5a209 Compare December 20, 2015 04:06

mbj force-pushed the fix/rubyspec-location branch from 7b5a209 to 94fd574 Compare December 20, 2015 04:27

mbj added the maintenance label Dec 20, 2015

mbj force-pushed the fix/rubyspec-location branch from 94fd574 to 8315a4d Compare December 20, 2015 17:57

This was referenced Dec 20, 2015

Reduce reference of IO globals #500

Merged

Add test minitest integration #445

Merged

Fix YARD docs

1d1cbb3

mbj force-pushed the fix/rubyspec-location branch 2 times, most recently from bcfd04a to 8315a4d Compare December 20, 2015 23:35

backus reviewed Dec 21, 2015
View reviewed changes

dkubb reviewed Dec 21, 2015
View reviewed changes

mbj force-pushed the fix/rubyspec-location branch from f90f541 to 49f807c Compare December 21, 2015 01:28

backus reviewed Dec 21, 2015
View reviewed changes

This was referenced Dec 25, 2015

Add mutation from Hash#values_at → Hash#fetch_values #510

Merged

Add mutations from Hash#[] and Hash#fetch to Hash#key? #511

Merged

mbj force-pushed the fix/rubyspec-location branch 2 times, most recently from 25f393f to 0bbf264 Compare December 30, 2015 18:42

backus reviewed Dec 30, 2015
View reviewed changes

mbj force-pushed the fix/rubyspec-location branch from 31ee858 to dbf7869 Compare December 30, 2015 20:26

dkubb reviewed Dec 31, 2015
View reviewed changes

mbj force-pushed the fix/rubyspec-location branch from be2720f to fe1deba Compare December 31, 2015 13:41

mbj force-pushed the fix/rubyspec-location branch from 3bcf025 to 1d1cbb3 Compare December 31, 2015 14:04

dkubb assigned mbj Dec 31, 2015

dkubb added a commit that referenced this pull request Dec 31, 2015

Merge pull request #508 from mbj/fix/rubyspec-location

bb4d4d9

Fix repo location of rubyspec

dkubb merged commit bb4d4d9 into master Dec 31, 2015

dkubb deleted the fix/rubyspec-location branch December 31, 2015 17:51

backus mentioned this pull request Jan 16, 2016

[WIP] Allow running along parser 2.3 mbj/unparser#60

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix repo location of rubyspec #508

Fix repo location of rubyspec #508

mbj commented Dec 19, 2015

mbj commented Dec 20, 2015

backus Dec 21, 2015

mbj Dec 21, 2015

backus commented Dec 21, 2015

dkubb Dec 21, 2015

mbj Dec 21, 2015

mbj commented Dec 21, 2015

backus Dec 21, 2015

mbj Dec 21, 2015

backus Dec 21, 2015

dkubb commented Dec 28, 2015

mbj commented Dec 29, 2015

dkubb commented Dec 29, 2015

mbj commented Dec 30, 2015

backus Dec 30, 2015

mbj Dec 30, 2015

mbj commented Dec 30, 2015

backus commented Dec 30, 2015

dkubb Dec 31, 2015

mbj Dec 31, 2015

mbj commented Dec 31, 2015

mbj commented Dec 31, 2015

Fix repo location of rubyspec #508

Fix repo location of rubyspec #508

Conversation

mbj commented Dec 19, 2015

mbj commented Dec 20, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

backus commented Dec 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbj commented Dec 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkubb commented Dec 28, 2015

mbj commented Dec 29, 2015

dkubb commented Dec 29, 2015

mbj commented Dec 30, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbj commented Dec 30, 2015

backus commented Dec 30, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbj commented Dec 31, 2015

mbj commented Dec 31, 2015