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

fix: slow post message #623

Merged
merged 6 commits into from
Aug 31, 2021
Merged

fix: slow post message #623

merged 6 commits into from
Aug 31, 2021

Conversation

2xAA
Copy link
Member

@2xAA 2xAA commented Aug 19, 2021

Limits meyda source input links for performance boost.
Adds new input link type "mutation" to further increase overall performance.

Adds a queue for syncing commits between the worker and renderer threads - sending lots of messages via postMessage is synchronous, it's better to send fewer messages with more content than lots of messages. we're talking around 40+ messages per 16ms here, so combining all these updates and syncing once per frame tick should help.

…orker and renderer threads

sending lots of messages via postMessage is synchronous, it's better to send fewer messages with
more content than lots of messages. we're talking around 40+ messages per 16ms here, so combining
all these updates and syncing once per frame tick should help
limits meyda source input links for performance boost. adds new input link type "mutation" to
further increase overall performance
@2xAA 2xAA requested a review from TimPietrusky August 19, 2021 12:34
Copy link
Member Author

@2xAA 2xAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to clear up some logging!

src/application/worker/index.worker.js Outdated Show resolved Hide resolved
src/application/worker/index.worker.js Outdated Show resolved Hide resolved
Copy link
Member

@TimPietrusky TimPietrusky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@2xAA thank you very much for this, I'm super happy that we could address this annoying issue.

I have some questions regarding the documentation and a general question.

I also wonder if we have to update these too regarding the new mutation type:

return value === mutationPayload[key];
});
} else {
payloadMatches = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this true if there is no payload?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it's possible for mutations to have no payload, or we just want to update when a mutation, e.g. beats/SET_KICK, is committed.

Example here:

this.$modV.store.dispatch("inputs/createInputLink", {
inputId: this.inputId,
type: "mutation",
location: "beats.kick",
match: {
type: "beats/SET_KICK"
},
source: "meyda",
args: [this.feature]
});


let payloadMatches = false;

if (match.payload) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is match?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

payloadMatches = true;
}

if (!payloadMatches) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have no payloadMatch, we continue? Why do we check if the payload is matching anyway? To see if there was a change?

Copy link
Member Author

@2xAA 2xAA Aug 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The payload may need to match to check for things like IDs or other unique parameters so we know the correct inputLink will be updated.

If we didn't check for the right ID on the MIDI input links, all MIDI input links would update on any MIDI input which would be wasteful in terms of postMessage.

inputs: { inputs, inputLinks }
} = store.state;

// Update mutation type Input Links
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is an input link of type mutation?

Copy link
Member Author

@2xAA 2xAA Aug 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are multiple types of input links.

  • all input links
    • Params:
      • inputId
        • String. The ID of the input to link to
      • ?source
        • String. Identifies what created and/or manages this input link
      • ?min
        • Number. Minimum expected value, will scale the incoming value to the input's min/max if min and max are set
      • ?max
        • Number. Maximum expected value, will scale the incoming value to the input's min/max if min and max are set
  • getter
    • ⚠️ updates every frame
    • Reads a getter in the store, possibly with arguments, defined in ?args, and applies that to the input link defined with inputId
    • Params:
      • location
        • String. location of the getter in the store, e.g. meyda/getFeature
      • ?args
        • Array. arguments for a getter that is a function, e.g. ["energy"]
  • mutation
    • Waits for a mutation type, defined with match.type, possibly checks the mutation payload for specific unique parameters, defined on match.?payload, and applies a store value, specified with location, to the input link defined with inputId
    • Params:
      • location
        • String. location of the value to read in the store, e.g. midi.devices[1].channelData[1][144]
      • match
        • Object. Parameters of the mutation to match. Has keys:
          • type
            • String. Mutation type from the store, e.g. midi/WRITE_DATA
          • ?payload
            • Object. Key-value pairs to match in the mutation payload. e.g.
            {
              id: `${id}-${name}-${manufacturer}`,
              channel,
              type
            } 
  • state
    • ⚠️ updates every frame
    • Reads a location in the store and applies that to the input link defined with inputId
    • Params:
      • location
        • String. location of the value to read in the store, e.g. midi.devices[1].channelData[1][144]

state and getter links are updated in loop.js

if (linkType === "getter" && linkArguments) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@2xAA thanks for this explanation!

src/application/worker/index.worker.js Outdated Show resolved Hide resolved
src/application/worker/loop.js Show resolved Hide resolved
@2xAA
Copy link
Member Author

2xAA commented Aug 19, 2021

@2xAA thank you very much for this, I'm super happy that we could address this annoying issue.

I have some questions regarding the documentation and a general question.

I also wonder if we have to update these too regarding the new mutation type:

@TimPietrusky Thanks for the review.

Expressions are handled independently of Input Links, they are already reactive.

Tweens could be updated, but they update every frame anyway so I don't think there would be any performance gain.
We may want to end their updates when the module is not enabled though, like we did with meyda source Input Links.

@TimPietrusky
Copy link
Member

@2xAA I was just trying to convert my old preset to use mutation and I'm wondering if we actually need the inputLinks.*.location anymore? As the same data is also in match and I also couldn't find any code that is using the location. Should this be removed from the saved preset?

@2xAA
Copy link
Member Author

2xAA commented Aug 21, 2021

@2xAA I was just trying to convert my old preset to use mutation and I'm wondering if we actually need the inputLinks.*.location anymore? As the same data is also in match and I also couldn't find any code that is using the location. Should this be removed from the saved preset?

@TimPietrusky, inputLinks.*.location is required and for API consistency we should keep this.

To explain my choice on keeping location, the only other way we could implement this, that I can think of, is to specify the key within the payload to get the value from and write that to the Input.
Right now, not every mutation has an Object as the payload, so it may serve us well to just keep the same structure.
This would also limit the possibilities of Input Links in the future as you may want to read data from another location based on a totally different mutation type.

If you agree with this then I think I will write up the explainer in one of the previous comments as JSDoc and we can go ahead and merge?

@TimPietrusky
Copy link
Member

@2xAA yes, that would be super perfect! Thanks ❤️

Copy link
Member

@TimPietrusky TimPietrusky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much <3

@TimPietrusky TimPietrusky merged commit cab2b82 into next Aug 31, 2021
@TimPietrusky TimPietrusky deleted the fix/slowPostMessage branch August 31, 2021 20:16
2xAA pushed a commit that referenced this pull request Nov 9, 2021
# [3.19.0](3.18.0...3.19.0) (2021-11-09)

### Bug Fixes

* **color-picker:** fixes colorPicker ([#631](#631)) ([293e223](293e223))
* **font-list:** updates dependancy ([#624](#624)) ([5aeecae](5aeecae))
* **gallery:** fixes gallery item preview ([#629](#629)) ([3272fbc](3272fbc)), closes [#597](#597) [#526](#526)
* **groups:** fixes default value for groups inheritFrom ([#628](#628)) ([d80f5ef](d80f5ef)), closes [#627](#627)
* **loop:** adds context to update method ([#641](#641)) ([1178273](1178273))
* **output-window:** Set proper encoding ([#634](#634)) ([41335b3](41335b3))
* **prop-default:** updates default prop check ([#646](#646)) ([0bc4e10](0bc4e10)), closes [#644](#644)
* **rangecontrol:** fixes an infinite update loop ([#643](#643)) ([334e024](334e024))
* **release:** use node 14 ([dd05d88](dd05d88))
* removes menuBar from appearing on output windows ([#632](#632)) ([5d6ef6a](5d6ef6a))
* slow post message ([#623](#623)) ([cab2b82](cab2b82))
* **slow-start-up:** refactors worker/modv lifecycle and module loading ([#618](#618)) ([ea8a15d](ea8a15d)), closes [#617](#617)
* **smoothing:** fixes inputLink smoothing ([#630](#630)) ([24864e2](24864e2))
* **splash-screen:** moves styles into main app.vue only ([#638](#638)) ([a90a09a](a90a09a))

### Features

* **isf:** remove unused error message & temp folder from media manager ([#636](#636)) ([4dfb299](4dfb299))
* **kick:** adds option for kick as inputlink and adds u_kick uniform for shaders ([#616](#616)) ([6aeac7f](6aeac7f))
* **offline-support:** Include raster.css + fonts ([#622](#622)) ([ef217bb](ef217bb)), closes [#621](#621)

### Performance Improvements

* **module-inspector:** changes v-for key from id to index ([#625](#625)) ([c79cb81](c79cb81))
@2xAA
Copy link
Member Author

2xAA commented Nov 9, 2021

🎉 This PR is included in version 3.19.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@2xAA 2xAA added the released label Nov 9, 2021
2xAA pushed a commit that referenced this pull request Jan 7, 2022
# [3.19.0](3.18.0...3.19.0) (2021-11-09)

### Bug Fixes

* **color-picker:** fixes colorPicker ([#631](#631)) ([293e223](293e223))
* **font-list:** updates dependancy ([#624](#624)) ([5aeecae](5aeecae))
* **gallery:** fixes gallery item preview ([#629](#629)) ([3272fbc](3272fbc)), closes [#597](#597) [#526](#526)
* **groups:** fixes default value for groups inheritFrom ([#628](#628)) ([d80f5ef](d80f5ef)), closes [#627](#627)
* **loop:** adds context to update method ([#641](#641)) ([1178273](1178273))
* **output-window:** Set proper encoding ([#634](#634)) ([41335b3](41335b3))
* **prop-default:** updates default prop check ([#646](#646)) ([0bc4e10](0bc4e10)), closes [#644](#644)
* **rangecontrol:** fixes an infinite update loop ([#643](#643)) ([334e024](334e024))
* **release:** use node 14 ([dd05d88](dd05d88))
* removes menuBar from appearing on output windows ([#632](#632)) ([5d6ef6a](5d6ef6a))
* slow post message ([#623](#623)) ([cab2b82](cab2b82))
* **slow-start-up:** refactors worker/modv lifecycle and module loading ([#618](#618)) ([ea8a15d](ea8a15d)), closes [#617](#617)
* **smoothing:** fixes inputLink smoothing ([#630](#630)) ([24864e2](24864e2))
* **splash-screen:** moves styles into main app.vue only ([#638](#638)) ([a90a09a](a90a09a))

### Features

* **isf:** remove unused error message & temp folder from media manager ([#636](#636)) ([4dfb299](4dfb299))
* **kick:** adds option for kick as inputlink and adds u_kick uniform for shaders ([#616](#616)) ([6aeac7f](6aeac7f))
* **offline-support:** Include raster.css + fonts ([#622](#622)) ([ef217bb](ef217bb)), closes [#621](#621)

### Performance Improvements

* **module-inspector:** changes v-for key from id to index ([#625](#625)) ([c79cb81](c79cb81))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants