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

Poor performance on debounce #41

Open
bbarker opened this issue Feb 27, 2020 · 5 comments
Open

Poor performance on debounce #41

bbarker opened this issue Feb 27, 2020 · 5 comments
Assignees

Comments

@bbarker
Copy link
Contributor

bbarker commented Feb 27, 2020

I looked into debounce a bit more as things still seemed laggy (debounce for text input fields set to 500ms, though trying 2000 did not seem to make a difference).

Over at https://labordynamicsinstitute.github.io/metajelo-ui/, things are fine when just a few input forms are open. But if you click on "Add item" for Supplementary products, then start typing in any text field (by holding down a single key), you'll probably see something that looks like this in Firefox:

image

Similar for Chrome. I haven't yet gotten react-devtools working with the build, but what I can say from looking at chrome's profiling is that there are many calls to React.DOM, and from Firefox's profiling, is that lots of non-incremental GCs are happening during text input, and I'm not sure why that would be.

Unsure if related to #40.

@bbarker
Copy link
Contributor Author

bbarker commented Feb 28, 2020

Just to confirm that it is actually a problem with debounce, I was going to do some logging;

-- TODO: remove the first two arguments from textInput', textInput, and urlInput
textInput' :: CtrlSignal HTML String
textInput' initVal = sig initVal
  where
    -- sig txt = debounce 5000.0 txt textInputWidget
    sig txt = do
      newTxt <- debounce 5000.0 txt textInputWidget
      sigEffPut $ log newTxt
      pure newTxt

-- For pure side effects
sigEffPut :: Effect Unit -> Signal HTML Unit
sigEffPut e = step unit do
  liftEffect e
  pure (step unit empty)

However, commenting out the existing debounce line and adding what you see above compiles but causes an infinite loop at runtime (i.e. page doesn't load).

@ajnsit ajnsit self-assigned this Feb 28, 2020
@bbarker
Copy link
Contributor Author

bbarker commented Feb 28, 2020

Two other points of data:

Also confirmed that loopW has the same apparent infinite-loop as debounce:

textInput' :: CtrlSignal HTML String
textInput' initVal = sig initVal
  where
    -- sig txt = debounce 5000.0 txt textInputWidget
    sig :: CtrlSignal HTML String
    sig txt = do
      -- newTxt <- debounce 5000.0 txt textInputWidget
      newTxt <- loopW txt textInputWidget
      sigEffPut $ log newTxt
      pure newTxt

Removing the sigEffPut $ log newTxt line stops that issue from happening, though I'm not sure how I should log text output in that case.

  1. Using debounce does not seem to have any pefrormance improvement over loopW in this case.

@bbarker
Copy link
Contributor Author

bbarker commented Feb 28, 2020

I copied out debounce from concur and stuck it into my source file ... and the performance issue is gone (though there is another issue which I may come back to in another issue thread later).

So this is odd because I verified (on two separate computers) that I was using the correct (latest) concur-core in my packages.dhall file, and I verified the code for debounce appeared correct in the .spago directory of my project.

@bbarker
Copy link
Contributor Author

bbarker commented Feb 28, 2020

After doing a more thorough clean in my project, performance is noticeably better on one system and I can tell debounce is outperforming loopW, though performance is still taking a hit just from entering text. (will try the other computer in a few hours for sanity's sake).

@bbarker
Copy link
Contributor Author

bbarker commented Feb 28, 2020

Some final notes (for now) after testing this out on the system I originally observed it on (Linux, compared to the other WIndows system, both quite beefy):

This is all after doing npm run clean

  1. There, adding in the definition of debounce did not seem to make a difference.
  2. The performance was generally fine in Chrome unless doing profiling, and in Firefox, the performance was sadly not as good, but acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants