-
Notifications
You must be signed in to change notification settings - Fork 308
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Faster and slicker Semantic Highlighting
The former implementation of semantic highlighting was slow becuase of the following two reasons: First, the annotations created by the semantic highlighting component, where invalidated and recomputed every time the compilation unit was reconciled. Second, for the current implementation of the AnnotationModel, adding/removing an annotation is O(n^ 2). (The quadratic behavior is due to the implementation of ``CompilationUnitDocumentProvider#ReverseMap``, which is used by ``CompilationUnitDocumentProvider#CompilationUnitAnnotationModel#addAnnotation``. Basically, each time an annotation is added/removed, the ``ReverseMap`` class - which is backed by a linked list - is accessed to verify if the element exist.) The proposed solution is to apply semantic colorings directly on the editor's ``TextPresentation`` and hence no longer use the ``AnnotationModel``. This approach is both lightweight and orders of magnitude more efficient than the former. The way semantic highlighting works now is relatively simple. At a high level, there are only three key elements: * ``Presenter$SemanticHighlightingJob``: Given a compilation unit, it classifyies and collects all positions that need to be colored in the editor. * ``PositionsTracker``: Keeps track of currently highlighted positions in the editor. * ``TextPresentationEditorHighlighter$ApplyHighlightingTextPresentationChange``: Converts them into ``StyleRange``s, and applies the styles to the editor's ``TextPresentation``. Of course the actual implementation contains a bit more than just the above mentioned classes, but most of it is needed to correctly handle platform's events such as the swapping of the document input (see ``Presenter$DocumentSwapListener`). There is however one important optimization that is worth mentioning, which is done to reduce to a minimum the editor's region whose styles need to be refreshed. Basically, whenever the user is typing in the editor, all positions that are **after** the cursor should be invalidated. If you are editing the beginning of a big file, almost the whole editor's text presentation would need to be refreshed. Furthermore, the mentioned approach would cause colors to flicker because all highlighted positions **after** the edit would be deleted (and hence the colors would be removed), and the new positions would get colored only **after** the semantic highlighting job completes. This is clearly not ideal, and it turns out it can be easily avoided by registering a ``IPositionUpdater`` on the editor's document. The concrete implementation of the listener (i.e., ``Presenter$PositionUpdater``) takes care of shifting - through side-effect - all currently highlighted positions that are **after** the edited region. The net effect is that colors never flicker. Bottom line, with the current implementation semantic highlighting is: * Considerably faster. * Never flickers. * Testable. If you wonder why the semantically highlighted positions are not stored in the document's model, the answer is that I simply could not see any benefit of doing so (why? because the document synchronizes access to the underlying position's map when adding/removing positions). Furthermore, it is really easy to add the missing logic for storing the positions in the document's model, if in the future it turns out we need/want to do so. Fix #1001156 Other facts about this commit: * Added the needed infrastructure for registering reconciling listeners (i.e., ``IJavaReconcilingListener``) to the editor. * Created a ``UiThread`` trait needed for testing code that needs to be run within the UI Thread. Other tickets fixed by this commit: * Disabling semantic highlighting in the preferences now refreshes currently opened editors. Fixes #1001507 * Changing semantic highlighting style's preferences now refreshes the currently opened editors. Fixes #1001508 * Cache highlighting styles instead of creating the during ``TextPresentationEditorHighlighter.ApplyHighlightingTextPresentationChanges.applyTextPresentation``. This optimization has major consequence on the user's experience. I should also point out that ``ScalaSyntaxClass`` and ``SymbolTypes`` are quite messy and should be improved. However, I think this should be tackled in a different PR. Fixes #1001493, #1001489. (cherry picked from commit 9bfeb02)
- Loading branch information
Showing
46 changed files
with
1,389 additions
and
519 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
282 changes: 282 additions & 0 deletions
282
...ests/src/scala/tools/eclipse/semantichighlighting/SemanticHighlightingPositionsTest.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,282 @@ | ||
| package scala.tools.eclipse.semantichighlighting | ||
|
|
||
| import scala.tools.eclipse.EclipseUserSimulator | ||
| import scala.tools.eclipse.ScalaPlugin | ||
| import scala.tools.eclipse.ScalaProject | ||
| import scala.tools.eclipse.javaelements.ScalaCompilationUnit | ||
| import scala.tools.eclipse.jface.text.EmptyRegion | ||
| import scala.tools.eclipse.properties.syntaxcolouring.ScalaSyntaxClasses | ||
| import scala.tools.eclipse.semantichighlighting.classifier.SymbolTypes | ||
| import scala.tools.eclipse.ui.InteractiveCompilationUnitEditor | ||
| import scala.tools.eclipse.util.CurrentThread | ||
| import scala.tools.eclipse.util.EclipseUtils | ||
| import scala.util.matching.Regex | ||
| import scala.util.matching.Regex.Match | ||
|
|
||
| import org.eclipse.core.internal.filebuffers.SynchronizableDocument | ||
| import org.eclipse.core.runtime.IProgressMonitor | ||
| import org.eclipse.core.runtime.IStatus | ||
| import org.eclipse.core.runtime.NullProgressMonitor | ||
| import org.eclipse.core.runtime.jobs.Job | ||
| import org.eclipse.jface.preference.IPreferenceStore | ||
| import org.eclipse.jface.text.IDocument | ||
| import org.eclipse.jface.text.IRegion | ||
| import org.eclipse.jface.text.Region | ||
| import org.eclipse.jface.text.source.ISourceViewer | ||
| import org.junit.{ Before, Test } | ||
| import org.junit.After | ||
| import org.junit.Assert | ||
| import org.mockito.Mockito._ | ||
|
|
||
| class SemanticHighlightingPositionsTest { | ||
| import SemanticHighlightingPositionsTest._ | ||
|
|
||
| private val MarkerRegex: Regex = """/\*\^\*/""".r | ||
| private val Marker = "/*^*/" | ||
|
|
||
| protected val simulator = new EclipseUserSimulator | ||
| private var project: ScalaProject = _ | ||
|
|
||
| private var sourceView: ISourceViewer = _ | ||
| private var document: IDocument = _ | ||
|
|
||
| private val preferences: Preferences = { | ||
| val store = mock(classOf[IPreferenceStore]) | ||
| when(store.getBoolean(ScalaSyntaxClasses.USE_SYNTACTIC_HINTS)).thenReturn(true) | ||
| new Preferences(store) | ||
| } | ||
|
|
||
| private var editor: TextPresentationStub = _ | ||
| private var presenter: Presenter = _ | ||
|
|
||
| private var testCode: String = _ | ||
| private var unit: ScalaCompilationUnit = _ | ||
| private var compilationUnitEditor: InteractiveCompilationUnitEditor = mock(classOf[InteractiveCompilationUnitEditor]) | ||
|
|
||
| @Before | ||
| def createProject() { | ||
| project = simulator.createProjectInWorkspace("semantic-highlighting-positions-update", true) | ||
| } | ||
|
|
||
| @After | ||
| def deleteProject() { | ||
| EclipseUtils.workspaceRunnableIn(ScalaPlugin.plugin.workspaceRoot.getWorkspace) { _ => | ||
| project.underlying.delete(true, null) | ||
| } | ||
| } | ||
|
|
||
| @Before | ||
| def setupMocks() { | ||
| sourceView = mock(classOf[ISourceViewer]) | ||
| document = new SynchronizableDocument | ||
| when(sourceView.getDocument()).thenReturn(document) | ||
| } | ||
|
|
||
| trait Edit { | ||
| def newText: String = "" | ||
| def newPositions: List[Position] = Nil | ||
| } | ||
| case class AddText(override val newText: String, override val newPositions: List[Position] = Nil) extends Edit | ||
| case object RemoveText extends Edit | ||
|
|
||
| private def setTestCode(code: String): Unit = { | ||
| testCode = code.stripMargin | ||
| val emptyPkg = simulator.createPackage("") | ||
| unit = simulator.createCompilationUnit(emptyPkg, "A.scala", testCode).asInstanceOf[ScalaCompilationUnit] | ||
| when(compilationUnitEditor.getInteractiveCompilationUnit).thenReturn(unit) | ||
| document.set(testCode) | ||
| } | ||
|
|
||
| private def createSemanticHighlightingPresenter(): Unit = { | ||
| editor = TextPresentationStub(sourceView) | ||
| presenter = new Presenter(compilationUnitEditor, editor, preferences, CurrentThread) | ||
| presenter.initialize(forceRefresh = false) | ||
| } | ||
|
|
||
| private def positionsInRegion(region: Region): List[Position] = { | ||
| val positions = editor.positionsTracker.positionsInRegion(region).filterNot(_.isDeleted()) | ||
| // We clone the `positions` because the semantic highlighting component works by performing side-effects | ||
| // on the existing positions. And, the `runTest`, expects the returned positions to not change. | ||
| positions.map(pos => new Position(pos.getOffset, pos.getLength, pos.kind, pos.deprecated)).toList | ||
| } | ||
|
|
||
| private def findAllMarkersIn(code: String): List[Match] = | ||
| MarkerRegex.findAllIn(code).matchData.toList | ||
|
|
||
| /** Test that semantic highlighting positions are correctly created for the passed `code` before | ||
| * and after performing the `edit`. It also checks that the damaged region caused by the `edit` | ||
| * is the smallest contiguous region that includes all positions affected by the `edit`. | ||
| * | ||
| * The passed `code` should contain at least one, and at most two, `Marker`. | ||
| * | ||
| * - One `Marker`: the new text carried by the `edit` should replace the `Marker` in | ||
| * the passed `code`. | ||
| * | ||
| * - Two `Marker`'s: the new text carried by the `edit` should replace the whole text contained | ||
| * within the two `Marker`s in the passed `code`. | ||
| * | ||
| * @param edit The edit action to perform on the passed `code`. | ||
| * @param code The test code. | ||
| */ | ||
| private def runTest(edit: Edit)(code: String): Unit = { | ||
| setTestCode(code) | ||
|
|
||
| createSemanticHighlightingPresenter() | ||
|
|
||
| val highlightedBeforeEdition = positionsInRegion(new Region(0, code.length)) | ||
|
|
||
| val markers = findAllMarkersIn(testCode) | ||
|
|
||
| val (start, end) = markers.size match { | ||
| case 1 => (markers.head.start, markers.head.end) | ||
| case 2 => (markers.head.start, markers.last.end) | ||
| case size => throw new AssertionError("Unsupported test definition. Found %d occurrences of %s in test code:\n%s".format(size, Marker, testCode)) | ||
| } | ||
| val length = end - start | ||
|
|
||
| val positionShift = edit.newText.length - length | ||
| val expectedHighlightedPositionsAfterEdition: List[Position] = { | ||
| // collect all positions in the document that are expected to still be highlighted after the edition | ||
| val positions = highlightedBeforeEdition.filterNot { _.overlapsWith(start, length) } | ||
| // of the above positions, shift the ones that are affected by the edition | ||
| val shiftedPositions = positions.map { pos => | ||
| if (pos.getOffset() >= start) pos.setOffset(pos.getOffset() + positionShift) | ||
| pos | ||
| } | ||
| // merge together the shifted positions and any additional position that is expected to be created by the edit | ||
| val merged = (edit.newPositions ++ positions) | ||
| // Sort the position. This is needed because position's added to the document are expected to be sorted. | ||
| val sorted = merged.sorted | ||
| sorted | ||
| } | ||
|
|
||
| def editTestCode(offset: Int, length: Int, newText: String): Unit = { | ||
| document.replace(start, length, edit.newText) // triggers the IUpdatePosition listener | ||
| unit.getBuffer().replace(start, length, edit.newText) // needed by the semantic highlighting reconciler | ||
| } | ||
|
|
||
| // perform edit | ||
| editTestCode(start, length, edit.newText) | ||
|
|
||
| // checks edit's postcondition | ||
| val currentTestCode = unit.getContents.mkString | ||
| if (findAllMarkersIn(currentTestCode).nonEmpty) | ||
| throw new AssertionError("After edition, no marker `%s` should be present in test code:\n%s".format(Marker, currentTestCode)) | ||
|
|
||
| // This will trigger semantic highlighting computation, which in turn will update the document's positions (sequential execution!) | ||
| editor.reconcileNow() | ||
|
|
||
| val newPositions = positionsInRegion(new Region(start, edit.newText.length())) | ||
| val affectedRegion = PositionsChange(newPositions, Nil).affectedRegion() | ||
|
|
||
| val highlightedAfterEdition = positionsInRegion(new Region(0, currentTestCode.length)) | ||
|
|
||
| Assert.assertEquals("Wrong start of damaged region", affectedRegion.getOffset(), editor.damagedRegion.getOffset()) | ||
| Assert.assertEquals("Wrong length of damaged region", affectedRegion.getLength(), editor.damagedRegion.getLength()) | ||
| Assert.assertEquals(expectedHighlightedPositionsAfterEdition.size, highlightedAfterEdition.size) | ||
| Assert.assertEquals(expectedHighlightedPositionsAfterEdition, highlightedAfterEdition) | ||
| } | ||
|
|
||
| @Test | ||
| def highlighted_positions_not_affected_by_edition() { | ||
| runTest(AddText("\n\n")) { | ||
| """ | ||
| |class A { | ||
| | def foo(): Unit = {} | ||
| | /*^*/ | ||
| |} | ||
| """ | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| def existing_highlighted_positions_are_shifted() { | ||
| runTest(AddText("\n\n")) { | ||
| """ | ||
| |class A { | ||
| | /*^*/ | ||
| | def foo(): Unit = {} | ||
| |} | ||
| """ | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| def new_highlighted_positions_are_reported() { | ||
| runTest(AddText("val bar = 2", List(new Position(17, 3, SymbolTypes.TemplateVal, deprecated = false)))) { | ||
| """ | ||
| |class A { | ||
| | /*^*/ | ||
| | def foo(): Unit = {} | ||
| |} | ||
| """ | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| def highlighted_positions_in_the_document_are_removed_on_deletion() { | ||
| runTest(RemoveText) { | ||
| """ | ||
| |class A { | ||
| | /*^*/ | ||
| | def foo(): Unit = {} | ||
| | /*^*/ | ||
| |} | ||
| """ | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| def highlighted_positions_around_deletion_action_are_correct() { | ||
| runTest(RemoveText) { | ||
| """ | ||
| |class A { | ||
| |/*^*/ | ||
| | if (true) | ||
| |/*^*/ println("abc") | ||
| |} | ||
| """ | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| def correctly_compute_damagedRegion_whenDeletingText() { | ||
| runTest(RemoveText) { | ||
| """ | ||
| |object A { | ||
| | /*^*/def f = 0 | ||
| | /*^*/class C | ||
| |} | ||
| """ | ||
| } | ||
| } | ||
| } | ||
|
|
||
| object SemanticHighlightingPositionsTest { | ||
|
|
||
| class TextPresentationStub(override val sourceViewer: ISourceViewer) extends TextPresentationHighlighter { | ||
| @volatile private var reconciler: Job = _ | ||
| @volatile var positionsTracker: PositionsTracker = _ | ||
| @volatile var damagedRegion: IRegion = _ | ||
|
|
||
| override def initialize(reconciler: Job, positionsTracker: PositionsTracker): Unit = { | ||
| this.reconciler = reconciler | ||
| this.positionsTracker = positionsTracker | ||
| reconcileNow() | ||
| } | ||
|
|
||
| override def dispose(): Unit = () | ||
|
|
||
| def reconcileNow(): Unit = { | ||
| damagedRegion = EmptyRegion | ||
| // `Job.run` is protected, but when we subclass it in `Presenter$Reconciler` we make the `run` method public, which is really useful for running the reconciler within the same thread of the test. | ||
| reconciler.asInstanceOf[{ def run(monitor: IProgressMonitor): IStatus }].run(new NullProgressMonitor) | ||
| } | ||
|
|
||
| override def updateTextPresentation(damage: IRegion): Unit = damagedRegion = damage | ||
| } | ||
|
|
||
| object TextPresentationStub { | ||
| def apply(sourceViewer: ISourceViewer): TextPresentationStub = new TextPresentationStub(sourceViewer) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.