Skip to content

Make EDGES_WORK_BUFFER_SIZE constant public to bindings #1291

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

Merged
merged 1 commit into from
Mar 25, 2025
+10 −10
Merged

Make EDGES_WORK_BUFFER_SIZE constant public to bindings #1291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/plan/tracing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//! This module contains code useful for tracing,
//! i.e. visiting the reachable objects by traversing all or part of an object graph.

use crate::scheduler::gc_work::{ProcessEdgesWork, SlotOf, EDGES_WORK_BUFFER_SIZE};
use crate::scheduler::{GCWorker, WorkBucketStage};
use crate::scheduler::gc_work::{ProcessEdgesWork, SlotOf};
use crate::scheduler::{GCWorker, WorkBucketStage, EDGES_WORK_BUFFER_SIZE};
use crate::util::ObjectReference;
use crate::vm::SlotVisitor;

Expand Down
8 changes: 0 additions & 8 deletions src/scheduler/gc_work.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,6 @@ use crate::*;
use std::marker::PhantomData;
use std::ops::{Deref, DerefMut};

/// Buffer size for [`ProcessEdgesWork`] work packets. This constant is exposed to binding
/// developers so that they can use this value for places in their binding that interface with the
/// work packet system, specifically the transitive closure via `ProcessEdgesWork` work packets
/// such as roots gathering code or weak reference processing. In order to have better load
/// balancing, it is recommended that binding developers use this constant to split work up into
/// different work packets.
pub const EDGES_WORK_BUFFER_SIZE: usize = 4096;

pub struct ScheduleCollection;

impl<VM: VMBinding> GCWork<VM> for ScheduleCollection {
Expand Down
8 changes: 8 additions & 0 deletions src/scheduler/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
//! A general scheduler implementation. MMTk uses it to schedule GC-related work.
/// Buffer size for [`ProcessEdgesWork`] work packets. This constant is exposed to binding
/// developers so that they can use this value for places in their binding that interface with the
/// work packet system, specifically the transitive closure via `ProcessEdgesWork` work packets
/// such as roots gathering code or weak reference processing. In order to have better load
/// balancing, it is recommended that binding developers use this constant to split work up into
/// different work packets.
pub const EDGES_WORK_BUFFER_SIZE: usize = 4096;
Copy link
Member

@qinsoon qinsoon Mar 25, 2025

Choose a reason for hiding this comment

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

You can just re-export it. See the pub use below.

Copy link
Collaborator Author

@k-sareen k-sareen Mar 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I thought it'd be better to actually have all constants together in mod.rs like we do for many other cases like in immix/mod.rs for example.

Copy link
Member

@qinsoon qinsoon Mar 25, 2025

Choose a reason for hiding this comment

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

The title suggested that you just wanted to make the constant public to bindings. I was suggesting pub use is the straightforward way to achieve that.

Copy link
Collaborator Author

@k-sareen k-sareen Mar 25, 2025

Choose a reason for hiding this comment

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

Right. But I thought it made sense to move the constant so that if we ever have other scheduler constants, they are not scattered around the different files. Up to you. I can move it back and re-export

Copy link
Member

@qinsoon qinsoon Mar 25, 2025

Choose a reason for hiding this comment

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

It looks okay. I think the difference is insigifnicant whether we put the constants in the top-level modules or lower-level submodules. There is no convention to require either way so it is your choice.


pub(crate) mod affinity;

#[allow(clippy::module_inception)]
Expand Down
Loading