Skip to content

fix(sockets movement): smoother throttling + fix re-render bug#541

Merged
waleedlatif1 merged 2 commits intostagingfrom
fix/sockets-movement
Jun 25, 2025
Merged

fix(sockets movement): smoother throttling + fix re-render bug#541
waleedlatif1 merged 2 commits intostagingfrom
fix/sockets-movement

Conversation

@icecrasher321
Copy link
Copy Markdown
Collaborator

Description

Throttling set to 120 FPS. And use collaborative update only on user actions.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • Performance improvement

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • All tests pass locally and in CI (bun run test)
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • I have updated version numbers as needed (if needed)
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Security Considerations:

  • My changes do not introduce any new security vulnerabilities
  • I have considered the security implications of my changes

Additional Information:

Any additional information, configuration or data that might be necessary to reproduce the issue or use the feature.

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 25, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sim ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 25, 2025 3:44am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
docs ⬜️ Skipped (Inspect) Jun 25, 2025 3:44am

@delve-auditor
Copy link
Copy Markdown

delve-auditor Bot commented Jun 25, 2025

No security or compliance issues detected. Reviewed everything up to 81cbdd8.

Security Overview
  • 🔎 Scanned files: 2 changed file(s)
Detected Code Changes
Change Type Relevant files
Enhancement ► workflow.tsx
    Improve collaborative block position updates
    Optimize node position handling
► socket-context.tsx
    Enhance real-time cursor and position update throttling

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Implements performance optimizations for collaborative workflow editing with improved socket movement handling and throttling mechanisms.

  • Increased socket throttling from 60fps to 120fps (16ms to 8ms) in apps/sim/contexts/socket-context.tsx for smoother real-time updates
  • Replaced interval-based throttling with optimized timeout-based approach in socket context to reduce memory overhead
  • Added explicit store version of updateBlockPosition in apps/sim/app/w/[id]/workflow.tsx to prevent collaborative editing feedback loops
  • Implemented collaborative position updates during node dragging for better multi-user synchronization
  • Fixed workflow switching state management to properly handle variable states

2 files reviewed, 2 comments
Edit PR Review Bot Settings | Greptile

Comment on lines +349 to 362
// Check if we already have a pending timeout for this block
if (!positionUpdateTimeouts.current.has(blockId)) {
// Schedule emission with light throttling (120fps = ~8ms)
const timeoutId = window.setTimeout(() => {
const latestUpdate = pendingPositionUpdates.current.get(blockId)
if (latestUpdate) {
socket.emit('workflow-operation', latestUpdate)
pendingPositionUpdates.current.delete(blockId)
} else {
// No more updates pending - stop the interval
clearInterval(intervalId)
positionUpdateTimeouts.current.delete(blockId)
}
}, THROTTLE_DELAY)

positionUpdateTimeouts.current.set(blockId, intervalId)
positionUpdateTimeouts.current.delete(blockId)
}, 8) // 120fps for smooth movement

// Set a cleanup timeout to stop the interval if no updates come in
setTimeout(() => {
if (positionUpdateTimeouts.current.get(blockId) === intervalId) {
clearInterval(intervalId)
positionUpdateTimeouts.current.delete(blockId)
pendingPositionUpdates.current.delete(blockId)
}
}, 50) // Stop interval after 50ms of no updates
positionUpdateTimeouts.current.set(blockId, timeoutId)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

style: Consider using requestAnimationFrame instead of setTimeout for better performance and synchronization with browser render cycles

Comment on lines +405 to 409
// Very light throttling at 120fps (8ms) to prevent excessive spam
if (now - lastCursorEmit.current >= 8) {
socket.emit('cursor-update', { cursor })
lastCursorEmit.current = now
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

style: Use a config constant for the throttle delay (8ms) to maintain consistency with other timing values

@vercel vercel Bot temporarily deployed to Preview – docs June 25, 2025 03:40 Inactive
@waleedlatif1 waleedlatif1 merged commit 0a5bf5a into staging Jun 25, 2025
4 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/sockets-movement branch June 28, 2025 04:46
arenadeveloper02 pushed a commit to arenadeveloper02/p2-sim that referenced this pull request Sep 19, 2025
* fix(sockets movement): smoother throttling strategy and re-render bug fix

* works

---------

Co-authored-by: Vikhyath Mondreti <vikhyathmondreti@Vikhyaths-Air.attlocal.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants