search

BinaryQuantumSoul / sdnext-modernui

SD.Next ModernUI
GNU Affero General Public License v3.0
16 stars 8 forks source link

refactor #6

Closed vladmandic closed 3 months ago

vladmandic commented 3 months ago

overview

changes

profiling

anyhow, startup time of uiux is now below 1sec (and it used to be >4sec!) which is acceptable
90% of time is in:

BinaryQuantumSoul commented 3 months ago

Really cool to finally have someone collaborating on the code!

Nice features!

Questions

Issues

Suggestions (check commit)

function functionLogTime( func ) { function newFunc() { const t0 = performance.now(); const returnValue = func(...arguments); const t1 = performance.now();

    console.log(`Done ${func.name}`, Math.round(t1 - t0) / 1000);
    return returnValue;
}

return newFunc;

}

test = functionLogTime(test); test(10);


- Refactor flags
```js
//= ====================== READY STATES =======================
function functionWaitForFlag(checkFlag) {
  return async function () {
    return new Promise((resolve) => {
      const check = () => checkFlag() ? resolve() : setTimeout(check);
      check();
    });
  }
}

let uiFlagInitialized = false;
let uiFlagPortalInitialized = false;

waitForUiReady = functionWaitForFlag(() => uiFlagInitialized);
waitForUiPortal = functionWaitForFlag(() => uiFlagPortalInitialized);

Profiling

vladmandic commented 3 months ago

What are the added core sdnext features ?

massive changes overall, but at the end not all that much that impacts uiux - onUiReady is a new event callback and log() function is updated to work with or without modernui.

I am not sure about renaming the file modernUI.js, won't we loose file history ?

true. you can ignore the pr and do the comit yourself.

  1. copy & paste content of modernui.js to sdnext_uiux_core.js and commit without modernui.js
  2. rename sdnext_uiux_core.js to modernui.js

What is the point of eslint formatting ? (it is really ugly)

formatting is always an opinionated thing - those are rules i've been using for a long time. but at the end, formatting is a sideeffect, more importantly eslint is there primarily as linter not as formatter. and it did catch some items that needed to be addressed plus some bad items (incorrect indent, undeclared variables, redeclared variables, blank spaces, mixed usage of tabs and spaces, etc.), although nothing really broken or horrible.

So now setupLogger is never called ? I can remove it, but we need to fix splash (see later) The splashScreen is hidden under gradio-app before all ui is loaded leaving a gray screen

correct. and yes, you can fix the splash so its never created.

Refactor flags

variable is not defined in global context unless defined on window level. function is. so main sdnext startup.js cannot use them as you wrote. but if you prefer your way, fine by me, just be aware that you need to make changes to startup.js as well.

It seems like logPrettyPrint is not using the same colors as prior.

you can reintroduce some colors, its just that it was linked to some really strange string patterns. i tried to make it match strings that are actually used in logging.

I think new favicon is way better than the old one, until we change the old one I want to keep this as it's fitting modern theme

i like the new favicon as well, but i really don't want to have different icon only when uiux is running. nothing preventing me from using new icon from right now - if you agree, i'll add it to core today.

Instead of copy-pasting time counter everywhere, do a function decorator

i know that, but while doing dev work, i made faaaar more timers within functions (t0, t1, t2, t3, t4) so i can trace each op. at the end i removed all interim ones that were below 0.1sec so what's left is what you see. and decorator has one sideeffect that it makes breakpoints/errorhandling/tracing much more difficult since it will appear in the decorator function instead in the desired function. all ok, just i prefer simpler approach while still in active dev.

Profiling

oh, wow, recursive fetch is really slow on your system. once this is merged, we can look at optimizing it - i have some ideas on how to parallelize at least a bit.

BinaryQuantumSoul commented 3 months ago

true. you can ignore the pr and do the comit yourself.

1. copy & paste content of `modernui.js` to `sdnext_uiux_core.js` and commit without `modernui.js`

2. rename `sdnext_uiux_core.js` to `modernui.js`

Will do

formatting is always an opinionated thing - those are rules i've been using for a long time. but at the end, formatting is a sideeffect, more importantly eslint is there primarily as linter not as formatter. and it did catch some items that needed to be addressed plus some bad items (incorrect indent, undeclared variables, redeclared variables, blank spaces, mixed usage of tabs and spaces, etc.), although nothing really broken or horrible.

How do I use it ?

correct. and yes, you can fix the splash so its never created.

It's already not created, but it's a z-index issue I'll fix

i like the new favicon as well, but i really don't want to have different icon only when uiux is running. nothing preventing me from using new icon from right now - if you agree, i'll add it to core today.

Go for it

oh, wow, recursive fetch is really slow on your system. once this is merged, we can look at optimizing it - i have some ideas on how to parallelize at least a bit.

What do you call recursive fetch ? Anyway we can optimize, but I think priority is fixing existing bugs

vladmandic commented 3 months ago

How do I use it ?

copy .eslintrc.json and package.json from sdnext root and do npm install - then you can just run eslint
sdnext is configured not to lint anything in extensions, but it will automatically check anything in core
so once we integrate uiux from extension into core, its best to be ready

Go for it

done.

What do you call recursive fetch ?

it loads initial template, then parses it then loads whatever is referenced there, etc.

Anyway we can optimize, but I think priority is fixing existing bugs

absolutely. none of the remaining performance items are critical.