We re-create the whole breakpoints side panel every time we step,
including the display of all the breakpoints even if they don't change.
One expensive part of doing this is calculating a preview of the source
line for which the breakpoint is set. To do this we need to search the
source file for all line endings, which on very large files can be
very expensive.
Line endings are currently cached on the TextUtils.Text.Text object.
We never get a chance to use this cached information though, because
the whole _breakpoints ListModel is re-created from a new data model
on each update, which we trigger on every Step.
Ideally we would only regenerate the UI that actually changed which
would avoid this recalculation on every step, but that is a bigger
change that I'm not confident we can make right now without potentially
breaking something.
Instead, when (re)creating the breakpoint items for the ListModel,
create and share the Text item between multiple breakpoints within the
same source file. Now we do O(files with breakpoints set) line-ending
calculations instead of O(breakpoints set).
This would still be very slow if you had hundreds of large files with
one breakpoint set in each, but I assume that is a way less common
scenario.
Line ending calculation is only about 50% of the cost of stepping at
the moment, so this speeds up by a factor of 2x but still leaves more
work to do (see screenshots of performance trace).
Before: https://imgur.com/a/qOYHi3u
After: https://imgur.com/a/8ydiuLO
Bug: 1069694
Change-Id: I8d8baf955ef71ab12dc85d5e8103a8bf598ba7da
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2156351
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
This CL changes references to self.Common.settings (the global
instance of SDK.Common.Settings) over to
Common.Settings.Settings.instance(). To keep both TypeScript and
Closure happy we must make a method on the Settings class itself,
since it only allows private constructors to be accessed by static
methods on the class.
Bug: 1058320
Change-Id: I04afc8caf64acf29cdda13ef03ad05cfff4786a1
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2091450
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
The source editor, when it cannot load a particular file, will simply
display a blank file instead. At the bottom, this is because the APIs
used to load the files do not have a way to propagate an error up to
the call site. Rather, the callback either is never called, or is
called with just an empty string.
This refactor changes the way the project system loads file contents
by replacing the callback model with a Promise-based model. However,
in this version, rather than propagating an error (handled via
catch), the error is exposed as a property on the object passed via
the the load functions. Although it might be preferable to use
async throw/catch, because there are ~4-5 layers of redirection
through the project system, the added complexity seems to not really
justify that work. I'm open to reconsidering this design, though.
Attempting to load a file via file:// which does not exist previously
produced no error because the DevToolsUIBindings handler would just
always resolve with no content and HTTP status 200. I had previously
addressed that bug in this changeset, but I've split it out to
https://chromium-review.googlesource.com/c/chromium/src/+/1847833 .
Sample "after" screenshot: https://imgur.com/a/tlm90sg
Bug: 961940
Bug: 941035
Change-Id: If121611090e9c35eeb1de162b59f8a9f72f696d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1817677
Reviewed-by: Lorne Mitchell <lomitch@microsoft.com>
Reviewed-by: Jeff Fisher <jeffish@microsoft.com>
Commit-Queue: Lorne Mitchell <lomitch@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#705438}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 61ec2a233d9a7bf4bb4b3e610de6fc233ae0d46e
The Chromium/Google style guides does not enforce curly braces for
single-line if-statements, but does strongly recommend doing so. Adding
braces will improve code readability, by visually separating code
blocks. This will also prevent issues where accidental additions are
pushed to the "else"-clause instead of in the if-block.
This CL also updates the presubmit `eslint` to run the fix with the
correct configuration. It will now fix all issues it can fix.
Change-Id: I4b616f21a99393f168dec743c0bcbdc7f5db04a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1821526
Commit-Queue: Tim Van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Jeff Fisher <jeffish@microsoft.com>
Cr-Original-Commit-Position: refs/heads/master@{#701070}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 7e0bdbe2d7f9fc2386bfaefda3cc29c66ccc18f9
The new compiler caught a lot of pre-existing issues in the codebase.
Sadly, the old compiler version was not smart enough to understand the
new changes. Therefore, the changes have be included in the same CL as
the compiler update.
Most of the changes are related to better handling of prototype and
class inheritance, as well as handling of null/undefined tracking.
Change-Id: I3941a3a240a4d09c4945e1e20d2521090ef837c9
Bug: 991710
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1762081
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@google.com>
Auto-Submit: Tim van der Lippe <tvanderlippe@google.com>
Cr-Original-Commit-Position: refs/heads/master@{#696761}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: ca93474213278e32e36d6ace1474c56884030757
Without this CL breakpoints manager listens for UISourceCodeRemoved
and ProjectRemoved events to reset locations when UISourceCode is gone.
It should use live locations instead.
Drive-by: fixed bug with breakpoints when UISourceCode with formatted
source is gone.
R=lushnikov@chromium.org
Bug: none
Change-Id: I3d23ff9e1ba7452d5e005cbc74e27119cda6eac7
Reviewed-on: https://chromium-review.googlesource.com/1178223
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Andrey Lushnikov <lushnikov@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#583891}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 89a6d413f8be13918811a13e835db75caa71c2c3
Tim van der Lippe
