Project: skia Issues People Development process History Sign in
New issue
Advanced search Search tips
Starred by 3 users
Status: Accepted
Owner:
Cc:
Area: PathOps
Priority: Medium
Type: Defect



Sign in to add a comment
Valgrind bot not running threaded PathOps tests.
Project Member Reported by bore...@google.com, Oct 14 2014 Back to list
The valgrind bot is timing out due to not producing output for 20 minutes in DM.  I suspect the easy solution is to just make it produce some output if it hasn't printed anything in over a minute or so.
 
Project Member Comment 1 by bore...@google.com, Oct 14 2014
... or, we could disable the threaded tests which are taking so long.
Project Member Comment 2 by bore...@google.com, Oct 15 2014
Labels: -BreakingTheBuildbots
Looks like it's no longer timing out after Mike committed https://chromium.googlesource.com/chromium/tools/build/+/b7c7eb5040dda43be2c52388e1aeadac4afefeff to disable the threaded tests.
Project Member Comment 3 by mtkl...@google.com, Oct 15 2014
Cc: bore...@google.com
Labels: -Priority-High Priority-Medium TestDisabled
Owner: caryclark@google.com
Summary: Valgrind bot not running threaded PathOps tests. (was: Valgrind bot times out)
Cary, are you okay with the Valgrind bot not running the threaded PathOps tests?  At least one of them runs so slowly under Valgrind that the new build system think's DM's hung and kills it.  Is there a go-quickly flag?

Here's what's being skipped today (via --match ~Threaded):
  PathOpsSimplifyQuadsThreaded
  PathOpsSimplifyRectsThreaded
  PathOpsSimplifyQuadralateralsThreaded
  PathOpsSimplifyDegeneratesThreaded
  PathOpsSimplifyTrianglesThreaded
  PathOpsRectsThreaded
  PathOpsOpCubicsThreaded
  PathOpsQuadLineIntersectionThreaded
Project Member Comment 4 by caryclark@google.com, Oct 15 2014
I've been experimenting with switching my long-winded tests to use a timer rather than a count -- e.g., most in tests/StrokerTest.cpp respect the new flag

   --timeout true

to stop iterating after 10 ms. (--timeout is set to true by default).

If everyone likes this pattern, I could switch all tests in pathops to use it.
Project Member Comment 5 by mtkl...@google.com, Oct 15 2014
Sounds good to me.
Project Member Comment 6 by hcm@google.com, Oct 15 2014
Labels: Area-PathOps
Project Member Comment 7 by caryclark@google.com, Aug 21 2015
Status: Fixed
Project Member Comment 8 by benjamin...@google.com, Nov 2 2016
Cc: benjamin...@google.com
Status: Accepted
Comments above make it sound like the PathOps.*Threaded tests should no longer be omitted on Valgrind, but there is still a match rule that omits them:
https://skia.googlesource.com/skia/+/96b38426b625b94489fe7ea237d79f9f9a5b406d/infra/bots/recipes/swarm_test.py#333
Project Member Comment 9 by caryclark@google.com, Nov 2 2016
Sounds good. I'll check that I am using the timeout everywhere I should.
Project Member Comment 10 by caryclark@google.com, Nov 2 2016
I can't find any trace of the old code. I assume that SkTaskGroup or the like handles this now?
Project Member Comment 11 by mtkl...@google.com, Nov 2 2016
Which old code?  What's the "this" that SkTaskGroup might be handling?
Project Member Comment 12 by caryclark@google.com, Nov 2 2016
Some time (years) ago, I added a command flag that, by default, checked a timer when running a threaded set of tests. If the tests took too long, the threaded test set was aborted, capping the number of tests run on slow machines. It was called FLAGS_timer.
 
Project Member Comment 13 by caryclark@google.com, Nov 2 2016
I got that wrong. FLAGS_timeout, which still exists in tests/StrokerTest.cpp. I thought that I had rewritten pathops test to use the same flag, but maybe I'm misremembering
Project Member Comment 14 by mtkl...@google.com, Nov 2 2016
Right.  I think you never did that.
Sign in to add a comment