Summary: | GC Activity Callback timer should be based on how much has been allocated since the last collection | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Hahnenberg <mhahnenberg> | ||||||||||||
Component: | JavaScriptCore | Assignee: | Mark Hahnenberg <mhahnenberg> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | benjamin, ggaren | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Mark Hahnenberg
2012-04-24 14:06:05 PDT
Created attachment 138638 [details]
Benchmark results for 10% CPU = 1 MB
Created attachment 138675 [details]
Benchmark results for 0.1%, 1%, and 10%
Created attachment 138689 [details]
Benchmark results with --measure-gc
Created attachment 138708 [details]
Patch
Comment on attachment 138708 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=138708&action=review > Source/JavaScriptCore/runtime/GCActivityCallbackCF.cpp:58 > +const double timerSlop = 2.0; // Fudge factor to avoid performance cost resetting timer. Typo: missing "of" before "resetting". > Source/JavaScriptCore/runtime/GCActivityCallbackCF.cpp:119 > + double gcTimeSlice = std::max((static_cast<double>(bytes) / MB) * gcTimeSlicePerMB, maxGCTimeSlice); Needs to be min(). Created attachment 138713 [details]
Patch
Comment on attachment 138713 [details]
Patch
r=me
Committed r115156: <http://trac.webkit.org/changeset/115156> Could this have caused the regression of Dromaeo/jslib-event-prototype? See http://webkit-perf.appspot.com/graph.html#tests=[[43017,2001,32196]]&sel=1335173610906.9148,1335421909212,1283.893395133256,1793.7427578215527&displayrange=7&datatype=running The changeset of this regression is http://trac.webkit.org/log/?rev=115156&stop_rev=115149&verbose=on (In reply to comment #10) > Could this have caused the regression of Dromaeo/jslib-event-prototype? > > See http://webkit-perf.appspot.com/graph.html#tests=[[43017,2001,32196]]&sel=1335173610906.9148,1335421909212,1283.893395133256,1793.7427578215527&displayrange=7&datatype=running > > The changeset of this regression is http://trac.webkit.org/log/?rev=115156&stop_rev=115149&verbose=on I'm confused by the graph on that page. What exactly is the y-axis? It appears to be doing more runs/second, which would be a progression, no? I feel like I'm missing something... > I'm confused by the graph on that page. What exactly is the y-axis? It appears to be doing more runs/second, which would be a progression, no? I feel like I'm missing something...
You are right. It is a test from Dromaeo.
Most bench are the other way around, in milliseconds. I was going over the regressions and I did not notice that particular one is Dromaeo.
Sorry for the wrong warning. Congrats on the progression! :)
(In reply to comment #12) > > I'm confused by the graph on that page. What exactly is the y-axis? It appears to be doing more runs/second, which would be a progression, no? I feel like I'm missing something... > > You are right. It is a test from Dromaeo. > > Most bench are the other way around, in milliseconds. I was going over the regressions and I did not notice that particular one is Dromaeo. > > Sorry for the wrong warning. Congrats on the progression! :) Thanks! I'm glad somebody is looking at the perf bot :-) Looks like a few of the other Dromaeo benchmarks had a decent progression around that time as well. Nice! Dromaeo wins on: http://webkit-perf.appspot.com/graph.html#tests=[[45011,2001,32196]]&sel=none&displayrange=7&datatype=running http://webkit-perf.appspot.com/graph.html#tests=[[43017,2001,32196]]&sel=none&displayrange=7&datatype=running http://webkit-perf.appspot.com/graph.html#tests=[[45016,2001,32196]]&sel=none&displayrange=7&datatype=running http://webkit-perf.appspot.com/graph.html#tests=[[39020,2001,32196]]&sel=none&displayrange=7&datatype=running http://webkit-perf.appspot.com/graph.html#tests=[[39019,2001,32196]]&sel=none&displayrange=7&datatype=running |