mozilla :: #devtools

20 Apr 2017
03:53bzAnyone around who's familiar with the watchpoint UI in the new debugger?
03:53* bz is running into a problem with it when updating to an HTML spec change, and isn't sure whether the problem is just our code or React in general (in which case the spec change needs to be undone)
07:04Honzarickychien: gasolin_ https://docs.google.com/document/d/1eTEEAcmT1Id3uT-fB1RP7Doll0rM27mW2MeA0bZyEAw/edit
08:18nchevobbeHonza: New reps bundle damp test
08:18nchevobbehttps://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=95befd0fcfdcd74d212cff8c227dae73cd94a6e7&newProject=try&newRevision=e08ae008b0423e4fef0ec6df4acee6c40558b327&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&filter=console&framework=1
08:19Honzanchevobbe: looks very green! :-)
08:19pbronchevobbe: wow the green
08:19pbrodoes that mean we're faster?
08:20nchevobbeHonza: there's clearly a performance improvement with messages that have a lot of reps, and also a bit with bulklog
08:20nchevobbepbro: yes and no
08:20nchevobbeso this is when comparing with the new frontendm without the new bundle
08:21nchevobbeand this is when we compare the new frontend with new bundle against the old frontend :
08:21nchevobbehttps://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=f1a0b6a6586e401a3cf6c6a65327ac9eabaeae03&newProject=try&newRevision=e08ae008b0423e4fef0ec6df4acee6c40558b327&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&filter=console&framework=1
08:21nchevobbeso we're still pretty far from what we used to have
08:21pbroso new reps are faster than old reps, when using a lot of them in the new console
08:22nchevobbethe 0.6 reps bundle is faster than the 0.5 one, yes
08:22pbrowell, that's good news already!
08:22nchevobbebecause we only use functions, and not components
08:23nchevobbeyes, it's a step in the good direction
08:23pbrowith the old frontend vs. new frontend tests, I don't understand the repstresstest
08:23pbrothe old console does not use reps, right?
08:23nchevobbeyeah, that's not really reps that we test
08:23nchevobbejust output lots of object in the same message
08:24pbrook. Since that's seems to be the thing that regressed the most, it's good that you're making that part better
08:24pbrowhat are your future plans for better console perf?
08:25nchevobbebrian is working on making the new frontend lives in its own html doc
08:25nchevobbesince for now it still lives in the old xul one, where we swap the old frontend with the new one
08:26nchevobbeit might gets us some benefits
08:27nchevobbealso, for the reps thing, i'm not sure that the stress test is really a common thing. I'm logging 20 complex variable in the same message, i've never done something like that when debuggin an app
08:27nchevobbethe major bottleneck for me is startup with cached messages
08:27pbrook
08:27pbrothat seems to make sense. Startup perf is the first impression users will get of the tool
08:28nchevobbeit's really bad, and I have to try some things to see how we can perform better
08:28nchevobbeyep
08:28pbroand might skew their perception, even if it's really fast afterwards
08:28pbrohow will the xul -> html document help with regards to perf?
08:28pbroI mean, we need to do it anyway, just curious if there are things you know about already
08:28nchevobbenot sure, but it's unclear if it impact performance or not
08:29pbrook
08:29pbroand about startup perf, is perf.html helpful to track down what the most problematic things are?
08:29nchevobbewell, for startup with cache it's a bit strange
08:30nchevobbeI see some huge gaps where nothing seems to happen
08:31nchevobbewe plan to chat with greg, see if he can help us spot clear perf issues
08:32nchevobbepbro: if you're curious https://perf-html.io/public/d2e1bb0dbc1585989a7134593b787d45551d81a7/calltree/?jsOnly&range=3.2805_12.6238&thread=0
08:32pbrooh cool, I saw some discussions about that
08:32nchevobbesee the gap between 4 and 6.5s
08:32pbrothanks for the profile
08:32nchevobbeno clue what's happening there
08:33pbrohmm, one trick I tried with the inspector is using console.mark("I'm here!") in various places (with unique names), then in perf.html, use the markers view (which is, for now, just a flat list of entries). That view gives you timestamps. So with that, you'd be able to map your markers with that long gap you're seeing
08:33pbromaybe the perf tool doesn't show you everything that's happening
08:33pbroso the custom markers would give you some indications of what the console is doing
08:39nchevobbepbro: oh nice, let me try that
08:39pbroyou'd have to add markers a bit everywhere though, sort of randomly :)
08:40nchevobbeyeah, i think it can help :)
08:47pbronchevobbe: so, how did you record this profile exactly: just opening the console? With lots of messages?
08:49nchevobbepbro: I run a function that logged 10000 messages (https://bgrins.github.io/devtools-demos/console/stress.html , LOL button), and then opened the console
08:50nchevobbepbro: so you can see in the content thread when we get the cached messages (which also takes quite some time ~500ms)
08:50nchevobbeand then the gap
10:09soleLOL button I love it nchevobbe
10:10nchevobbesole: bgrins created this :) I love it too
10:10soleit's the funny little things...
10:11pbroyeah, made me LOL
10:14nchevobbeFirefox DevTlols
10:15soleDevTrololols
10:15nchevobbeahah, this is good
10:28pbrodon't mess with the spelling. Remember, it's dEVtoOls
10:33soleimagining jryans twisting in his sleep and he doesn't even know why
10:33sole"Wow I had a terrible sleep this night"
10:34soleWakes up and sees all these evil casing variations 8-)
10:34pbrohaha. Having said that, I'm also very careful with writing DevTools carefully
10:34soleSo I heard WordPress have a plugin that will ensure every time "WordPress" is in the content of a post, the casing is correct
10:35solebecause Matt mullenweg was like jryans
10:35solehehehe
10:35pbronice
10:38pbronchevobbe: I just did a recording of the console opening with the stress test page, and I'm not seeing this huge gap you have
10:39nchevobbeCan you send me the profile link ?
10:40pbrohttps://perfht.ml/2outqGX
10:40pbronchevobbe: it starts on the main thread with the opening of the toolbox (the 10000 console.logs are not captured)
10:40pbrothen the content thread seems to be doing all the actor setup
10:41pbrothen back to the main thread with something like 400ms of, what I think is, console rendering
10:41pbrothen another 300ms on the content thread with more console related work
10:53pbrothis is all the startup part really. Then I see a huge 4sec for creating all the React components. Followed by 1s of actual react rendering. And followed by one hue 600ms reflow marker
10:53pbroI mean, nothing really surprising. But I do not see the gap with no data that you had
10:54pbroI'll look at this more later. Got to go.
11:33freddybhey, people of #devtools, would you be opposed if I enabled the no-implied-eval rule in your eslintrc?
11:33freddybit not causing any errors as of now
11:56solefor the lazies http://eslint.org/docs/rules/no-implied-eval
11:56sole(it seems fine for me)
11:56solewe probably shouldn't be evalling I guess
11:57soleI mean not evalling in that way, probably scratchpad or the console need "some sort" of evaluation :D
11:57freddybyeah, eslintrc says devtools has their other way, which make sense
11:58freddybI'm sure nobody wants to disallow that :)
11:58freddybsole: thx for providing the link :)
11:58soleno probs!
11:59nchevobbepbro: interresting, i did it again with a clean profile and not seeing the gap anymore
11:59freddybI'm enable the rule in the main eslintrc in bug https://bugzilla.mozilla.org/show_bug.cgi?id=1358050
11:59firebotBug 1358050 ASSIGNED, fbraun@mozilla.com Add no-implied-eval rule to eslint config
11:59freddybsole: who would be a good person to approve in the bug, if I wanted to include the change for devtools' eslintrc within that change?
12:02nchevobbejdescottes: blazing fast review ! Thanks :)
12:09jdescottesnchevobbe: np,thanks for doing the release :P
12:10nchevobbejdescottes: sure :)
12:18jryansfreddyb: tromey is probably a good reviewer for this
12:18nchevobbejdescottes: I filed Bug 1358103 for removing the createFactory calls. I'll do one commit per tool and ask for review to owners
12:18firebothttps://bugzil.la/1358103 ASSIGNED, nchevobbe@mozilla.com Remove createFactory call on Reps
12:18freddybjryans: would you suggest doing it in the same bug or in its own?
12:19jryansfreddyb: either way is fine i tihkn
12:19jdescottesnchevobbe: great! not sure it's worth splitting in several commits though? it should be a pretty straightforward change in each file right?
12:19freddybok, I'll move this forward later tonight (CEST). thanks jryans
12:20nchevobbejdescottes: yeah, it should be. I'll see how much changes it implies and see if it's worth splitting
12:21jdescottes
13:12wedrIs there a way to force a stack trace when ReferenceError has occurred? Can't seem to figure out where it is complaining that there's no then() when using Promises. http://imgur.com/j4gkYVw
13:15nchevobbewedr: this looks like a console.warn(error.message) that could have been caught in a try/catch
13:16wedrYeah, but I have no idea where that is being caught/uncaught.
13:18wedrThe main issue is the debugger isn't displaying where exactly it is being called from the extension, and would instead display where it is actually called both from the extension and from other JS libraries.
13:19wedrIt can be argued that's the intended behavior, but I wanted to be able to force the DOMException errors to do a full stack trace.
13:21wedrGreat, doing a simple project search for the console.warn(), the fact that it was caught in jQuery, meant there's more issues in the extension codes than I hoped.
13:43jryansochameau: hmm, is there a notification when the active docshell changes?
14:00ochameaujryans: I'm not sure, at least I don't see any obvious notification from here:
14:00ochameauhttp://searchfox.org/mozilla-central/source/docshell/base/nsDocShell.cpp#6217
14:00jryansochameau: i just spotted visibilitychange, it may work
14:01jryansochameau: seems to get fired by SetIsActive
14:01ochameaujryans: great!
14:04tromeyclarkbw: ping
14:04clarkbwtromey: hello
14:04tromeyclarkbw: so if I'm reading that email correctly, there's no need for the repack work, and I ought to close the repack bug?
14:05tromeyclarkbw: or is that just a statement of releng wishes and not a final decision?
14:05clarkbwtromey: i don't think there's a decision there, but it seems that the repack isn't the best option anymore
14:05tromeyok, thanks. I will wait for the decision before closing the bug
14:07jlastsole - DevTrolls are the worst!
14:08clarkbwwha ha ha
14:08soleDev
14:33jlastpbro, gl - do you know where the Rule View finds the <style> elements in the page?
14:33jlastI&#39;m looking for a simple way to find <script> tags in the page
14:34tromeythe rule view actually doesn&#39;t search for them all, I think, instead it finds the element based on a style sheet
14:34tromeysort of backwards from what you want I guess
14:35jlastthat&#39;s a good start as the most specific use case is finding the script we&#39;re paused in
14:36tromeyI felt like it was in devtools/server/actors/stylesheets.js but I&#39;m not finding it right now
14:38tromeyI guess it just looks at sheet.ownerNode
14:51pbrothe style-editor does that though. I iterates through all frames in a window, and then uses a platform API (DOMUtils.getAllStyleSheets) to get all stylesheets
14:51pbroif you were looking for a particular tag, you could do that too I guess
14:52jlastI see. Thanks
14:52pbrobut as tromey said, the rule-view doesn&#39;t need to find the <style> elements in a page. It lists the CSS rules that apply to a given element
14:53jlastoh i see, that is different
14:54jlastit looks like it uses document.stylesheets where it can: http://searchfox.org/mozilla-central/source/devtools/server/actors/stylesheets.js#903
14:54jlastwhich is cheating :)
14:55tromeyyeah
14:55tromeybut the existence of the markup view shows that the script nodes can be found, if that&#39;s what you want to do
14:56pbroI thought the debugger had a platform API to list all scripts though?
14:56tromeyyeah, but the scripts can be GCd
14:56tromeyand I think jlast wants something that isn&#39;t GCd
14:56tromeybut then a <script> can be removed from the DOM
14:57pbrook,but <script>s wouldn&#39;t give you things that are eval&#39;d for instance
14:57tromeythat too
14:58jlastpbro - that is okay. We just need to make sense of what is in the initial HTML file
14:58jlastso that if you pause there we can preview variables on hover or search for functions
14:59pbrojlast: ok, then I guess walking the DOM (including child frames) is what you need. You could either use standard DOM APIs, or a documentWalker like the inspector does
14:59tromeythere are also things like onload=&quot;this is evalled&quot;
14:59pbronot sure what the characteristics of the DocumentWalker are vs. just using querySelectorAll and such
15:00jlasthaha - true I always do <link onload=&quot;debugger&quot;>
15:01jlastpbro i&#39;m worried that the current DOM might be different than the source text we receive
15:02jlastHmm, I&#39;ll do some investigation and report back
15:29pbrojlast: ok sounds good. Let me know if you want help on the dom tree walker we use in the inspector, maybe that&#39;s something you want to try
15:29jlastthanks
15:39Honzahttps://drive.google.com/drive/folders/0B52XT6_ifv2ESGc3aEgxLVJ5YWM?ths=true
15:47ochameaugregtatum: ping
16:15nchevobbebgrins, Honza : https://bugzilla.mozilla.org/show_bug.cgi?id=1358180
16:15firebotBug 1358180 NEW, nobody@mozilla.org Add DAMP tests for the console
16:15nchevobbepatch is coming soon
16:15bgrinsthanks
16:27nchevobbejryans: thanks for the comment on the bug. I already wrote the tests and used them in perfherder, so it&#39;s only a matter of having them somewhere so we can track our progress. We&#39;re focusing on performance these days, so I think (and hope), that the day we move to github we fixed the bottlenecks in the console
16:28jryansnchevobbe: okay, great, then it sounds reasonable to proceed :)
16:29jryansnchevobbe: keep this nonsense in mind when updating damp: https://bugzilla.mozilla.org/show_bug.cgi?id=1343774#c25
16:29firebotBug 1343774 FIXED, gasolin@mozilla.com remove unused functions
16:30bzAnyone around familiar with the watchpoint UI?
16:30* bz is running into test failures with it when implementing some HTML spec changes, and isn&#39;t sure what the right fix is or should be
16:32gregtatumochameau: pong
16:35solebz: maybe jlast
16:35jlastHey bz
16:35bzjlast: hey. So brief summary of the problem
16:35bzjlast: when we have a watchpoint for the expression &quot;f&quot;
16:35bzjlast: when the user double-clicks on the watchpoint
16:36bzjlast: we end up with a textfield containing the text &quot;f&quot; and a text cursor in it, right?
16:36bzjlast: So the user can edit the expression.
16:36ochameaugregtatum: it looks like on windows (or may be it is relate to being on nightly build instead of custom artifact builds?), I do not see the chrome JS in the content process in the call tree
16:36ochameaugregtatum: typically, I do not see actor code in the profiler
16:37jlastsorry, are you referring to &quot;watch expressions&quot;?
16:37gregtatumochameau: that&#39;s odd
16:37bzjlast: yes
16:37jlastif so, correct :)
16:37bzjlast: ok.
16:37bzjlast: So the question is where that text cursor is. Is it before the &quot;f&quot; or after it?
16:37bzjlast: Right now, it&#39;s after, due to a quirk in React and its interaction with a part of the HTML spec that people want to change.
16:37gregtatumochameau: not sure how that could happen, do you have an example profile?
16:37jlasthmm, interesting. let me check
16:38jlasti believe after is where we would prefer it
16:38bzjlast: and we have tests relying on that: devtools/client/debugger/new/test/mochitest/browser_dbg-expressions.js does a simulated double-click on an &quot;f&quot; watch expression, simulates typing &quot;oo&quot; and expects the result to be a watch for &quot;foo&quot;
16:38bzOK
16:38bzSo if we make the HTML spec change, it would end up before
16:38jlastoh i see - yeah.
16:38bzUnless we ourselves position it after.
16:38jlastwe can change the test honestly to double click and then move the cursor
16:39bzWell, that&#39;s the question
16:39bzShould we change the test, or should we change our double-click impl?
16:39bzIn terms of the UX we want to expose to users
16:40jlastThe UX is a low priority at the moment, so lets do the easiest thing possible :)
16:40jlastWe can easily go back and change the UX on the react side later.
16:40bzok
16:40bzThanks!
16:40jlastI can update the test now to simulate cmd+rightarrow.
16:41bzOh, awesome.
16:41jlastwhen do you think the test will fail?
16:41bzIf you can attach the test fix to https://bugzilla.mozilla.org/show_bug.cgi?id=1357206 I can take it from there
16:41firebotBug 1357206 NEW, nobody@mozilla.org Don&#39;t move selection to end when .value is set to its existing value
16:41bzIn terms of landing it
16:42bzdevtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-cond.js will need a similar fix, etc; I&#39;m happy to do all that once I have an example to crib. ;)
16:42ochameaugregtatum: sure, here is one on windows, where you don&#39;t see any actor running in content
16:42ochameaugregtatum: https://perfht.ml/2pHbP09
16:42bz(I could probably come up with a test fix myself too; just wanted to check whether we wanted to change the test at all or not)
16:43jlastgot it. sure, feel free to add the test fix too while you&#39;re working on the change. that might be simpler
16:43ochameaugregtatum: another one on linux, same actions, where you clearly see actors - https://perfht.ml/2oZQr8l
16:43jlastthanks for reaching out
16:46ochameaugregtatum: wait there is something weird. it actually depends on my actions.
16:47ochameaugregtatum: one test case is involving webgl+webasm, computing stuff while devtools is running
16:47gregtatumochameau: your buffer looks like it&#39;s dropping information
16:47ochameaugregtatum: the other doesn&#39;t. it is quiet, there is just devtools
16:48gregtatumochameau: oh wait, dur, you have filters
16:48ochameaugregtatum: this is surprising there isn&#39;t any frame ending up in devtools?
16:48ochameaufilters in the stacks search box?
16:48ochameauI don&#39;t have any
16:50gregtatumochameau: it&#39;s possible you&#39;re just not sampling when devtools is running code
16:50gregtatumochameau: plus it looks like your buffer dropped a bunch of samples
16:50ochameaugregtatum: to confirm, should I change interval and/or buffer size?
16:51gregtatumochameau: what&#39;s your buffer and interval set to?
16:51wedrjlast: Can you point to me in the debugger.html codebase, where do you handle the highlighting in the Watch list?
16:52gregtatumochameau: I would recommend restarting your browser, only have 1 tab open, and then run your test with an interval of 0.3ms, and a buffer >200mb
16:52ochameaugregtatum: default
16:52wedrIt&#39;s really annoying when the highlight just flashes briefly.
16:52wedrIf it can flash on/off repeatedly forever, that&#39;s a good idea.
16:52gregtatumochameau: 0.3ms gives you a better idea of timing, without breaking your page performance for the sample rate
16:53gregtatumochameau: really it just looks like your buffer got emptied for some reason, not sure what are all the cases when that happens, i have a few open bugs on empty buffers, i haven&#39;t looked into it though
16:53ochameaugregtatum: I&#39;m running with this very consuming demo - https://s3.amazonaws.com/mozilla-games/ZenGarden/EpicZenGarden.html
16:54gregtatumochameau: haha, yeah...
16:55gregtatumochameau: your code is taking 50-60ms per frame to run the scripts
16:55ochameaugregtatum: ok, I do see devtools with these settings! thanks a lot
16:55gregtatumochameau: the page&#39;s code is going to dominate the samples
16:55gregtatumochameau: ok, great!
16:55wedrjlast: This is what I&#39;m looking at, but I don&#39;t think that&#39;s the right code that controls how long the flashing hightlighted watched expressions are when the breakpoint was hit. https://github.com/devtools-html/debugger.html/blob/3066a9df26f7a7652454883741aa248c9086276d/src/components/SecondaryPanes/index.js#L92
17:35ochameaugregtatum: thx again for the profiler, just fixed a perf issue in less than 1 hour ;)
17:35gregtatumochameau: woot woot! glad to hear :)
17:52gregtatumhttps://irccloud.mozilla.com/file/ndio2HTb/
18:00jlasthey wedr one minute
18:01jlasthere&#39;s the component: https://github.com/devtools-html/debugger.html/blob/master/src/components/SecondaryPanes/Expressions.js#L1
18:01jlastgregtatum: what have you done :)
18:01gregtatumjlast: no idea :)
18:02jlastheh - nice
18:03jlastwedr - not sure what you mean by the highlighting
18:05wedrjlast: When the debugger hits a watched expression, the watched expression updates, along with a brief flash (orange colored row background). I want the flashing to stay persistent, instead of very briefly appearing and disappearing.
18:09wedrjlast: I didn&#39;t seem to find any code that controls the timing issue in the link provided. Does this mean the timing controls are handled internally?
18:09wedr:(
18:10jlasthmm - wedr. I think we actually fixed this bug :)
18:10jlastare you testing in nightly or using debugger.html locally?
18:11wedrjlast: Stable. The March 3rd release.
18:12jlasthttps://github.com/devtools-html/debugger.html/issues/2347
18:12wedrDon&#39;t want to risk doing stuff while I&#39;m actually using it in production code. :/
18:12jlastyep we fixed this 3/8 or so
18:13jlastwe pushed a new release today, so it should be on nightly tomorrow
18:13jlastthat was annoying :)
18:13wedrjlast: Ah, um, I was referring to something else.
18:14wedrHold on a minute, I&#39;m starting to hallucinate if FF dev edition is using debugger.html or not.
18:55tromeywhy would this forceStyle function be needed at all? https://dxr.mozilla.org/mozilla-central/source/devtools/client/shared/theme-switching.js#37
18:55tromeyI commented it out and afaict theme switching still works fine
18:57tromeyaha, I think it&#39;s subtle - this is needed to switch scrollbar styles
18:57tromeymore mysteries
19:15jryanstromey: maybe if we gave both themes floating scrollbars, it wouldn&#39;t be needed? i think some were in favor of it https://discourse.mozilla-community.org/t/scrollbar-differences-between-light-and-dark-themes/13403
19:16tromeyhaha way to cross the streams
19:16jryansoh, but i guess you were not, haha ;)
19:16tromeyno but you know
19:16tromeyI am curious to know why this is needed
19:17gregtatumhttps://irccloud.mozilla.com/file/xotIRn24/
19:17gregtatumReact is going to be getting a lot easier to profile here coming soon
19:18tromeywhat am I looking at?
19:18gregtatumThat&#39;s the performance.measure() information that dev builds of react record
19:18jryanstromey: hmm, maybe the style system doesn&#39;t restyle for new UA sheets automatically (since they are unexpected)
19:18gregtatumthis is all super raw
19:18tromeyaha cool gregtatum
19:18gregtatumI just instrumented performance.measure() on a local build
19:19tromeyjryans: ok, interesting. and we need UA sheets for some reason
19:19gregtatumAnd threw it at my marker view prototype
19:19tromeyso great
19:20jryanstromey: i think it&#39;s because the selectors target anonymous content you can&#39;t &quot;normally&quot; style
19:21tromeythanks for the explanation jryans
20:37bzjlast: ping
20:37jlastpong
20:38bzjlast: so are these tests basically code dumps from upstream somewhere?
20:38bzjlast: (just updated to tip and ran into conflicts because 1354672 got backed out in the meantime afaict)
20:38jlastthe tests are here: http://searchfox.org/mozilla-central/source/devtools/client/debugger/new/test/mochitest
20:39bzright
20:39bzI know that
20:39bzThe point is... if I update them there
20:39jlastwe do the development in github.com/devtools-html/debugger.html/, but if you make the changes in m-c we can backport it
20:39bzOK
20:39bzThat&#39;s the question. ;)
20:40bzThanks!
20:40jlastyup. the patch got repushed this morning because of l10n issues (hopefully will stick)
20:40bzoh, it&#39;s been repushed?
20:40bzah, to inbound
20:40bzwell, then
20:40* bz strips more stuff, updates
20:41bzThanks!
21 Apr 2017
No messages
   
Last message: 36 days and 17 hours ago