Skip to content

Conversation

jeremy
Copy link
Contributor

@jeremy jeremy commented May 24, 2022

Vips.concurrency_set(0) suggests it'll set concurrency back to the
default value, but it does nothing if concurrency is already set, which
is always true because Vips.init has already configured thread pools.

The result is that if you set concurrency to any value, setting it to
zero will leave that value in place.

This reads the default concurrency immediately after Vips.init so we
can make good on having concurrency_set 0 reset to default. Also
allows setting nil to reset to default.

Note that setting concurrency does not reconfigure existing thread
pools. It affects thread pools spawned for future operations.

References #335

Rakefile Outdated
@@ -1,5 +1,8 @@
require "bundler/gem_tasks"

# Disable stderr warning output
ENV["VIPS_WARNING"] = "1"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We purposefully test behaviors that warn, so disable stderr output.

attach_function :vips_concurrency_get, [], :int

# Track the original default concurrency so we can reset to it.
DEFAULT_CONCURRENCY = vips_concurrency_get
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Read the default right away so we can reset to it. (Note Vips.vips_init just above which sets the initial vips__concurrency to VIPS_CONCURRENCY or CPU count.)

# Get the default size of libvips worker pools.
def self.concurrency_default
DEFAULT_CONCURRENCY
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Useful to expose VIPS_CONCURRENCY or CPU count.

# var or the number of hardware threads on your computer.
def self.concurrency
vips_concurrency_get
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For inspection, logging, metrics, and the like.

def self.concurrency_set n
n = DEFAULT_CONCURRENCY if n.to_i == 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Assign 0 or nil to restore the initial default.

vips_concurrency_set n
concurrency
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice to return the value that was set.

# before Vips.init sets concurrency to the default.
DEFAULT_VIPS_CONCURRENCY = 5
ENV["VIPS_CONCURRENCY"] = DEFAULT_VIPS_CONCURRENCY.to_s

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For predictable testing. Set our own VIPS_CONCURRENCY rather than let it default to CPU count.

`Vips.concurrency_set(0)` suggests it'll set concurrency back to the
default value, but it does nothing if concurrency is already set, which
is always true because Vips.init has already configured thread pools.

The result is that if you set concurrency to any value, setting it to
zero will leave that value in place.

This reads the default concurrency immediately after Vips.init so we
can make good on having `concurrency_set 0` reset to default. Also
allows setting `nil` to reset to default.

Note that setting concurrency does not reconfigure existing thread
pools. It affects thread pools spawned for future operations.
@jeremy jeremy force-pushed the concurrency-default branch from 68e261f to 3e6cd18 Compare May 24, 2022 07:06
@jcupitt jcupitt merged commit ed6103c into libvips:master May 24, 2022
@jcupitt
Copy link
Member

jcupitt commented May 24, 2022

What a nice clean PR. Good job!

jcupitt added a commit that referenced this pull request May 24, 2022
@jcupitt
Copy link
Member

jcupitt commented May 24, 2022

... I updated the CHANGELOG.

@jeremy jeremy deleted the concurrency-default branch May 24, 2022 22:30
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