Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve LibraryManager Dialog rendering #10254

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ricardojlrufino
Copy link
Contributor

TableCellRender is designed to be reusable.
Te old implementation
created an instance, every time the mouse was over the line, This can lead to rendering problems, and low responsiveness

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Tests

I made a small benchmark with the results:

First run:

  • OLD: 423813 ms
  • New: 286341 ms

Second run:

  • OLD: 442275 ms
  • New: 277634 ms

Test Class..

package cc.arduino.test;

import java.awt.Rectangle;
import java.awt.Toolkit;

import javax.swing.JTable;
import javax.swing.SwingUtilities;

public class BenchmarkRenderer {
  static long benchmarkTable(final JTable list, String msg) {
    Runnable doScroll = new Runnable() {
      public void run() {
        int index = list.getSelectedRow();
        list.changeSelection(index + 1, 0, false, false);
        // make visible to trigger render...
        Rectangle cellRect = list.getCellRect(index + 1, 0, false);
        list.scrollRectToVisible(cellRect);
      }
    };

    Runnable doNothing = new Runnable() {
      public void run() {
      }
    };

    long startTime = System.currentTimeMillis();

    list.changeSelection(0, 0, false, false);

    /*
     * Each iteration of this loops queue a request to scroll the list on the
     * event dispatching thread (we're running on the "main" thread) and then
     * waits for all of the paint operations caused by the scroll to finish.
     * Finally we force any output that has been buffered by the AWT
     * implementation to be flushed (with Toolkit.sync()). Then overhead
     * introduced here is the same for each list so the elapsed time numbers can
     * be safely compared.
     */
    for (int i = 0; i < list.getModel().getRowCount(); i++) {
      try {
        SwingUtilities.invokeLater(doScroll);
        Thread.yield();
        SwingUtilities.invokeAndWait(doNothing);
        Toolkit.getDefaultToolkit().sync();
      } catch (Exception e) {
        e.printStackTrace();
      }
    }

    long endTime = System.currentTimeMillis();
    long time =  endTime - startTime;
    System.out.println(">>" + time);
    return time;
  }

  public static void main(String[] args) throws Exception {
   
  }
}

TableCellRender is designed to be reusable, the old implementation
created an instance, every time the mouse was over the line.
@cmaglie
Copy link
Member

cmaglie commented May 25, 2020

I tried to downgrade a library from 1.2.0 to 1.1.0 and I got:

     [exec] java.lang.RuntimeException: java.util.ConcurrentModificationException
     [exec] 	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:248)
     [exec] 	at java.lang.Thread.run(Thread.java:748)
     [exec] Caused by: java.util.ConcurrentModificationException
     [exec] 	at java.util.LinkedList$LLSpliterator.forEachRemaining(LinkedList.java:1239)
     [exec] 	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
     [exec] 	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
     [exec] 	at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
     [exec] 	at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
     [exec] 	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
     [exec] 	at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
     [exec] 	at cc.arduino.contributions.libraries.LibrariesIndexer.rescanLibraries(LibrariesIndexer.java:165)
     [exec] 	at cc.arduino.contributions.libraries.LibrariesIndexer.setLibrariesFolders(LibrariesIndexer.java:118)
     [exec] 	at processing.app.BaseNoGui.onBoardOrPortChange(BaseNoGui.java:677)
     [exec] 	at processing.app.Base.onBoardOrPortChange(Base.java:1346)
     [exec] 	at processing.app.Base$11.actionPerformed(Base.java:1578)
     [exec] 	at processing.app.Base.rebuildBoardsMenu(Base.java:1550)
     [exec] 	at processing.app.Base$8.onIndexesUpdated(Base.java:1378)
     [exec] 	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:246)
     [exec] 	... 1 more

after clicking "Upgrade" to bring it back to 1.2.0:

java.lang.RuntimeException: java.lang.NullPointerException
	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:248)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.NullPointerException
java.lang.NullPointerException
java.lang.RuntimeException: java.lang.NullPointerException
	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:248)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.NullPointerException
java.lang.NullPointerException
java.lang.RuntimeException: java.lang.NullPointerException
	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:248)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.NullPointerException
java.lang.NullPointerException
java.lang.RuntimeException: java.lang.NullPointerException
	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:248)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.NullPointerException
java.lang.NullPointerException
java.lang.RuntimeException: java.lang.NullPointerException
	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:248)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.NullPointerException
java.lang.NullPointerException
java.lang.RuntimeException: java.lang.NullPointerException
	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:248)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.NullPointerException
java.lang.NullPointerException
java.lang.RuntimeException: java.lang.NullPointerException
	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:248)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.NullPointerException
 Index: 0, Size: 0
java.lang.RuntimeException: java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:248)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
	at java.util.ArrayList.rangeCheck(ArrayList.java:657)
	at java.util.ArrayList.get(ArrayList.java:433)
	at processing.app.Base.rebuildImportMenu(Base.java:1141)
	at processing.app.Base$11.actionPerformed(Base.java:1579)
	at processing.app.Base.rebuildBoardsMenu(Base.java:1550)
	at processing.app.Base$8.onIndexesUpdated(Base.java:1378)
	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:246)
	... 1 more

BTW this doesn't always happen, I need to navigate and tinker a bit on the libraries list (i.e. select different version from menus, etc.).

EDIT: adding another stacktrace just for completeness, it may help or it maybe not...:

     [exec] java.lang.RuntimeException: java.lang.NullPointerException
     [exec] 	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:248)
     [exec] 	at java.lang.Thread.run(Thread.java:748)
     [exec] Caused by: java.lang.NullPointerException
     [exec] 	at cc.arduino.contributions.libraries.LibrariesIndexer.rescanLibraries(LibrariesIndexer.java:158)
     [exec] 	at cc.arduino.contributions.libraries.LibraryInstaller.rescanLibraryIndex(LibraryInstaller.java:207)
     [exec] 	at cc.arduino.contributions.libraries.LibraryInstaller.install(LibraryInstaller.java:129)
     [exec] 	at cc.arduino.contributions.libraries.LibraryInstaller.install(LibraryInstaller.java:117)
     [exec] 	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:239)
     [exec] 	... 1 more
     [exec] java.lang.NullPointerException
     [exec] java.lang.RuntimeException: java.lang.NullPointerException
     [exec] 	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:248)
     [exec] 	at java.lang.Thread.run(Thread.java:748)
     [exec] Caused by: java.lang.NullPointerException
     [exec] 	at cc.arduino.contributions.packages.ContributedPlatform.getResolvedTools(ContributedPlatform.java:113)
     [exec] 	at cc.arduino.contributions.packages.ContributedPackage.findResolvedTool(ContributedPackage.java:84)
     [exec] 	at cc.arduino.contributions.packages.ContributionsIndexer.syncToolWithFilesystem(ContributionsIndexer.java:316)
     [exec] 	at cc.arduino.contributions.packages.ContributionsIndexer.syncPackageWithFilesystem(ContributionsIndexer.java:307)
     [exec] 	at cc.arduino.contributions.packages.ContributionsIndexer.syncLocalPackages(ContributionsIndexer.java:286)
     [exec] 	at cc.arduino.contributions.packages.ContributionsIndexer.syncWithFilesystem(ContributionsIndexer.java:222)
     [exec] 	at processing.app.BaseNoGui.initPackages(BaseNoGui.java:491)
     [exec] 	at processing.app.Base$8.onIndexesUpdated(Base.java:1377)
     [exec] 	at cc.arduino.contributions.libraries.ui.LibraryManagerUI.lambda$onInstallPressed$4(LibraryManagerUI.java:246)
     [exec] 	... 1 more
     [exec]  Custom menu not found!

@ricardojlrufino
Copy link
Contributor Author

Strange, this commit should not be that behavior.
This had happened to me some of these when I was implementing the startup improvements and I found the error: #10228 , you have tested using #10270 ?

@cmaglie
Copy link
Member

cmaglie commented May 25, 2020

mmm no just tested again, I've checkout out this PR from git (so only this commit).
On the master branch the bug is not present, so it has to be something with this patch but, admiteddly, I can't figure out what...

@per1234 per1234 added the Component: Board/Lib Manager Boards Manager or Library Manager label Sep 30, 2020
@CLAassistant
Copy link

CLAassistant commented Apr 9, 2021

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Board/Lib Manager Boards Manager or Library Manager
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants