firstdraft / appdev_template

A Rails template for generating homework projects
0 stars 1 forks source link

Make consistent with minimal #157

Closed jelaniwoods closed 4 years ago

jelaniwoods commented 4 years ago

The new --minimal flag will remove

The proposed changes:

Why make puma single threaded?

The "Session Expired" better_errors can be resolved if we stick to a single threaded server in development. See https://github.com/BetterErrors/better_errors/issues/292#issuecomment-554461349

raghubetina commented 4 years ago

@jelaniwoods Ahhh amazing. Can't wait to look over this.

Question in the meantime: have you noticed any slowdowns from dropping Spring and/or Bootsnap? These two are invisible to students and as far as I can tell haven't caused any issues, so I might be in favor of keeping them, since they in theory provide a better development experience.

Separately, I might be in favor of keeping system tests as a forcing function on us to learn how to write them, because that seems to be the direction things are headed in.

raghubetina commented 4 years ago

The "Session Expired" better_errors can be resolved if we stick to a single threaded server in development. See BetterErrors/better_errors#292 (comment)

Awesome, nice find.

raghubetina commented 4 years ago

meta_request

Whoa I totally forgot about this gem. I agree that we can remove it for AppDev 1 projects, but in case you aren't familiar with it @jelaniwoods @pmckernin, I've found it handy in the past; it's intended to be used in conjunction with this:

https://github.com/dejan/rails_panel

jelaniwoods commented 4 years ago

have you noticed any slowdowns from dropping Spring and/or Bootsnap? These two are invisible to students and as far as I can tell haven't caused any issues, so I might be in favor of keeping them, since they in theory provide a better development experience.

@raghubetina I've maybe noticed a very slight increase (1-2 seconds) of time it takes for bin/server or rails grade but I don't know if that was the result of removing spring and bootsnap or just regular Gitpod-ness. It also might have been in foodhub, which is already slower grading b/c of selenium.

raghubetina commented 4 years ago

@jelaniwoods Hm do you think it would be easy to benchmark? It's not obvious to me how one would go about that... perhaps you could do a

`rspec`

from within a pure Ruby script and do a simple benchmark on it?

jelaniwoods commented 4 years ago

@raghubetina I'm not sure if this worked correctly;

require 'benchmark'
n = 5
Benchmark.bm(7) do |x|
  x.report("rails grade:")   { `rails grade` }
end

Without:

              user     system      total        real
rails grade:  0.000000   0.000819  12.867924 ( 14.349086)

With spring:

              user     system      total        real
rails grade:  0.000530   0.000150   8.035838 (  9.453474)
raghubetina commented 4 years ago

@jelaniwoods Hm, I haven't seen bm being used that way, but interesting. Since the code under test is a command line program that needs to be modified before the comparison, I can't think of a better way. Usually we include both versions of the code we want to compare within blocks using report, but we can't do that here.

Are these measurements from the project that has selenium in the specs? Ideally we're just isolating the impact of spring on the test startup time as much as possible, so I would test it on a brand new project that's as slim as possible; maybe just the one dummy spec, and use rspec instead of rails grade.

If the impact of spring is really ~5 seconds on every test run, then I think we should include it; that's a significant improvement to student ergonomics IMO. Thoughts?

jelaniwoods commented 4 years ago

The benchmark was done in omnicalc-1 without any modifications.

I tried testing a different way, still in omnicalc-1:

require 'benchmark'

puts Benchmark.measure { `rspec` }

I stripped the tests down to just one expect(1).to be 1, which is similar to the dummy_spec

and ran the benchmark three times each.

With spring

gitpod /workspace/omnicalc-1 $ ruby test.rb 
  0.000000   0.000810   2.275064 (  2.684595)
gitpod /workspace/omnicalc-1 $ ruby test.rb 
  0.000688   0.000000   2.211564 (  2.225482)
gitpod /workspace/omnicalc-1 $ ruby test.rb 
  0.000000   0.000641   2.265334 (  2.276773)

Without spring

gitpod /workspace/omnicalc-1 $ ruby test.rb 
  0.000653   0.000113   3.356090 (  3.382423)
gitpod /workspace/omnicalc-1 $ ruby test.rb 
  0.000496   0.000072   3.511029 (  3.541096)
gitpod /workspace/omnicalc-1 $ ruby test.rb 
  0.000000   0.000620   3.413625 (  3.441158)

which feels a little closer to reality than the first results.

spring is faster by around 1.0592756666 seconds

raghubetina commented 4 years ago

@jelaniwoods @pmckernin Have you ever had to resolve any issues with spring for students? I used to experience random issues that ended up being caused by spring, and then a bin/spring stop fixed it for reasons I never understood. However I can't recall that happening to me within, oh, a year or so; and I can't recall having to do it for students for multiple years.

If none of us have observed it causing issues then I'd say we can leave spring in for the slight speedup it provides. Thoughts?

raghubetina commented 4 years ago

@jelaniwoods Oh it just occurred to me that spring mostly speeds up second run and onwards, after it loads the application into memory; so perhaps you should run it twice, measuring the time it takes for the second run.

jelaniwoods commented 4 years ago

@raghubetina I agree. I can only recall one time when spring caused an issue but that was about a year ago soI assume it was patched.

I re-ran the benchmark twice after stopping spring to see the effects.

gitpod /workspace/omnicalc-1 $ bin/spring stop
Spring stopped.
gitpod /workspace/omnicalc-1 $ ruby test.rb 
  0.000011   0.001000   4.515039 (  4.955387)
gitpod /workspace/omnicalc-1 $ ruby test.rb 
  0.000027   0.000701   2.655432 (  2.677162)

which looks like about a ~2.4 second increase in speed.