Skip to content

Fewer hdf5 reads when loading precomputed meshes #8110

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 7, 2024
Merged

Fewer hdf5 reads when loading precomputed meshes #8110

merged 3 commits into from
Oct 7, 2024

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Oct 2, 2024 •
edited
Loading

Saves some reads both in the chunk list request and when reading actual chunk data

  • mostly don’t re-read constant attributes for every segment
  • allocate fewer java objects in merge step of list chunks request (no more reduce(mergeTwo))

URL of deployed dev instance (used for testing):

Steps to test:

  • Load precomputed meshes, both computed for a mapping, and computed for the oversegmentation, should still work
  • Especially when using an oversegmentation meshfile, but loading meshes with mapping (so it’s applied on-the-fly), there should be a speedup.

Measured Speedup

Loading a typical mesh (segment 25 in l4dense_motta_et_al_dev with mapping 90, using oversegmentation meshfile, so that the agglomerate consists of 10458 oversegments)

Measured on my laptop with SSD.

Chunk List Request

old new improvement
cold 1792 ms 950 ms 47 %
warm 1075 ms 642 ms 40 %

Chunk Request (average of the 80 chunks)

old new improvement
cold 29 ms 20 ms 31%
warm 14 ms 9 ms 35%
  • Updated changelog
  • Removed dev-only changes like prints and application.conf edits
  • Considered common edge cases
  • Needs datastore update after deployment

@fm3 fm3 self-assigned this Oct 2, 2024
@fm3 fm3 added the refactoring label Oct 2, 2024
@fm3 fm3 requested a review from MichaelBuessemeyer October 2, 2024 10:06
fm3 added a commit that referenced this pull request Oct 2, 2024
@fm3
remove backend changes from this PR, they are moved to #8110
c5c3e94
MichaelBuessemeyer
MichaelBuessemeyer approved these changes Oct 7, 2024
View reviewed changes
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

Everything looks good to me and the code seems to work. 🚀

I only left a few questions :)

Regarding the speed-up: I did not try to measure it myself. But it makes sense that the speed increased due to our changes :)

@fm3 fm3 merged commit 18fb4e0 into master Oct 7, 2024
2 checks passed
@fm3 fm3 deleted the meshfile-loading-perf branch October 7, 2024 10:51
philippotto added a commit that referenced this pull request Apr 9, 2025
@daniel-wer @fm3
@philippotto @MichaelBuessemeyer
Faster Mesh Rendering and Raycasting in 3D viewport (#8106)
dadaeb6 
* Upgrade threejs to v0.169. Add three-mesh-bvh library and use it to speed up raycasting.

* log timing

* log duration for raycaster

* save some backend perf in chunk list request

* Update BufferGeometryUtils. Merge all geometries of a precomputed mesh to optimize performance

* Merge all geometries of each mesh respectively

* throttling delay 0, weil das ja klar is

* Revert "throttling delay 0, weil das ja klar is"

This reverts commit 36f5c0f.

* remove logging in raycaster

* Revert "Merge all geometries of each mesh respectively"

This reverts commit 852fc2f.

* revert mesh merging

* fix mergeBufferGeometries

* fix mergeBufferGeometries

* Remove unnecessary react-debounce-render lib

* optimize chunkinfo merge, chunk request reads

* further optimize chunk loading reads

* restore file exists check

* Revert "revert mesh merging"

This reverts commit 65cbc5e.

* Reapply "Merge all geometries of each mesh respectively"

This reverts commit 5f279e7.

* Add window.DEBUG_BVH flag and don't use groups when merging geometries since a BVH is created for each group otherwise.

* less logging

* remove backend changes from this PR, they are moved to #8110

* small clean up

* fix colors and improve lighting

* fix aggressive pop up when deleting a single tree via context menu

* misc

* wip: use vertex colors to highlight parts of a mesh; use cumulative index to avoid linear scanning of attribute buffer

* add more todo comments

* fix selecting/highlighting super voxels in proofreading mode

* remove mergeChunks and isMerged booleans because these are always true now

* restore hover behavior for non-proofreading (whole mesh should be highlighted)

* implement webknossos.DEV.benchmarkRotate()

* clean up more isMerged stuff

* fix multiSelectMenu performance part of #7895 (which was already closed, but this particular thing wasn't fixed yet)

* use binary search instead of extra unmappedSegmentId BufferAttribute

* offload bvh computation to webworker

* fix hovering for ad hoc meshes

* more clean up (e.g., don't group by position anymore)

* fix changing mesh colors when segment color changes

* refactor

* more clean up

* restore progressive loading behavior (already add partial meshes during loading of precomputed mesh; remove parts again before adding merged mesh to scene)

* temporarily disable ci

* update mesh bvh to 0.9.0

* restore url hash parsing

* re-add throttling to avoid lags when meshes are not yet merged

* fix types in VisibilityAwareRaycaster

* fix typing

* re-enable ci

* update changelog

* format

* cherry pick ignoreWarnings from #8389

* bump webpack deps

* mock compute_bvh_async due to esm problems

* remove lil-gui

* fix cyclic dependencies

* lint

* incorporate some PR feedback

* inline setColor function

* remove gui

* clean up

* improve rotate benchmark so that actual measurements are done; remove shadow properties as it didn't do anything and also had a slight impact on performance

* remove raycasting perf measurement

* refactor getMeshVisibilityStateForSegments so that local state is not necessary

* clean up (remove opacity change on hover altogether, as it's not critical and felt weird to me)

* add assertions

* fix that mesh groups were arrays instead of dicts; ensure that meshes are removed before they are re-computed; await adding of mesh to scene to avoid that ad-hoc item is immediately removed from list

* add comment to PositionToSegmentId

* rename PositionToSegmentId to VertexSegmentMapping

* sort imports

* Apply suggestions from code review

Co-authored-by: MichaelBuessemeyer <[email protected]>

---------

Co-authored-by: Florian M <[email protected]>
Co-authored-by: Philipp Otto <[email protected]>
Co-authored-by: Philipp Otto <[email protected]>
Co-authored-by: MichaelBuessemeyer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaelBuessemeyer MichaelBuessemeyer MichaelBuessemeyer approved these changes

Assignees

@fm3 fm3

Labels

backend performance refactoring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@fm3 @MichaelBuessemeyer