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

Optimize the file watcher #148

Merged
merged 12 commits into from
Dec 30, 2023
Merged

Optimize the file watcher #148

merged 12 commits into from
Dec 30, 2023

Conversation

TheElectronWill
Copy link
Owner

@TheElectronWill TheElectronWill commented Aug 31, 2023

  • Use only one WatchService per filesystem
  • Add a configurable throttling delay to avoid reloading configuration too often (todo: make it a debounce instead)
  • Use WatchService#poll(timeout, unit) instead of poll() to wait for events instead of busy looping
  • and more improvements :)

Copy link

@comp500 comp500 left a comment

Choose a reason for hiding this comment

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

Changes look good to me, thanks for looking into this!

@solonovamax
Copy link

Is there any way this can get merged? It seems that night-config is hammering my filesystem which is, uh, Not Nice™

and if for whatever reason this can't be merged quickly, could a quick-and-dirty change at least be introduced that at adds Thread.sleep(1000L) in between checking if the config has updated?

@TheElectronWill
Copy link
Owner Author

TheElectronWill commented Nov 20, 2023

Yes! I'd like to add a bit more tests and then I'll merge it. Not so fan of a quick-and-dirty fix, the real one will be much better :p

@solonovamax
Copy link

Not so fan of a quick-and-dirty fix, the real one will be much better :p

I do agree with this, but it's really hammering my disk causing significant issues with other programs (where programs like IntelliJ take several times longer to load files than it should) unless I close minecraft. (I'm not entirely sure that this is the cause, but I believe that it is)
So, honestly, I'd love just to have a temporary solution in the meantime. (Though, I might just clone the branch and build it myself lol. but afaik a few of the mods I have shade it directly into the jar lol)

@TheElectronWill
Copy link
Owner Author

I do agree with this, but it's really hammering my disk causing significant issues with other programs (where programs like IntelliJ take several times longer to load files than it should) unless I close minecraft. (I'm not entirely sure that this is the cause, but I believe that it is)

How many configs do you have? Do you modify them while playing? The FileWatcher is watching for changes through the OS, it doesn't do much when there's no change 🤔

Inspecting the Minecraft process with a tool like VisualVM would provide some insights.

@solonovamax
Copy link

How many configs do you have? Do you modify them while playing? The FileWatcher is watching for changes through the OS, it doesn't do much when there's no change 🤔

It might not be the filewatcher causing this, could be smth else, but I just took an educated guess based on absolutely nothing, that this was the cause of this lol

@TheElectronWill TheElectronWill merged commit c7b2b30 into master Dec 30, 2023
1 check passed
0utplay pushed a commit to CloudNetService/CloudNet that referenced this pull request May 16, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[com.electronwill.night-config:json](https://togithub.com/TheElectronWill/night-config)
| `3.6.7` -> `3.7.1` |
[![age](https://developer.mend.io/api/mc/badges/age/maven/com.electronwill.night-config:json/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/com.electronwill.night-config:json/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/com.electronwill.night-config:json/3.6.7/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/com.electronwill.night-config:json/3.6.7/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
|
[com.electronwill.night-config:yaml](https://togithub.com/TheElectronWill/night-config)
| `3.6.7` -> `3.7.1` |
[![age](https://developer.mend.io/api/mc/badges/age/maven/com.electronwill.night-config:yaml/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/com.electronwill.night-config:yaml/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/com.electronwill.night-config:yaml/3.6.7/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/com.electronwill.night-config:yaml/3.6.7/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
|
[com.electronwill.night-config:toml](https://togithub.com/TheElectronWill/night-config)
| `3.6.7` -> `3.7.1` |
[![age](https://developer.mend.io/api/mc/badges/age/maven/com.electronwill.night-config:toml/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/com.electronwill.night-config:toml/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/com.electronwill.night-config:toml/3.6.7/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/com.electronwill.night-config:toml/3.6.7/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>TheElectronWill/night-config
(com.electronwill.night-config:json)</summary>

###
[`v3.7.1`](https://togithub.com/TheElectronWill/night-config/releases/tag/v3.7.1):
3.7.1 Gradle fix

##### Fixes

- Resolve a misconfiguration of the Gradle build, see
[TheElectronWill/night-config#173
for more details
-   Fix a minor issue with the new options of `JsonParser`

You should use v3.7.1 instead of v3.7.0.
Please [read the release notes of
v3.7.0](https://togithub.com/TheElectronWill/night-config/issues/173)
:sparkles: to learn what has changed since v3.6.x.

**Full Changelog**:
TheElectronWill/night-config@v3.7.0...v3.7.1

###
[`v3.7.0`](https://togithub.com/TheElectronWill/night-config/releases/tag/v3.7.0):
3.7.0: NightConfig rejuvenated ✨

##### Major changes

- **`FileWatcher` now comes with out-of-the-box debouncing**, which
improves the performance of autoreloading and autosaving configurations
:zap:, see
[TheElectronWill/night-config#148
- **`FileConfig` instances are now actually thread-safe** and work
better with autoreload, which should prevent most of the corruption
issues 🛡️, see
[TheElectronWill/night-config#152
- **A new Serialization/Deserialization framework** is available in the
`serde` package 🚀, see
[TheElectronWill/night-config#163.
Unlike the old `ObjectConverter`, the new `ObjectSerializer` and
`ObjectDeserializer` are modular and extensible: you can register your
own serializers and deserializers. Deserializers (config -> object) are
chosen based on the generic type of the field to set and the type of the
config value to deserialize. This allows for fine-grained
deserialization. The old `ObjectConverter` still works but is now
deprecated. The goal is to remove the old `conversion` package in
version 4.0.0.

##### Enhancements

- Allow multiple spec conditions to be checked by
[@&#8203;vaperion](https://togithub.com/vaperion) in
[TheElectronWill/night-config#151
(Note: this fix applies to the deprecated `conversion` package; the new
`serde` package supports multiple `@SerdeAssert` conditions out of the
box)
- Support UTF-16 and UTF-8 BOM by
[@&#8203;TheElectronWill](https://togithub.com/TheElectronWill) in
TheElectronWill/night-config@68502b9
- In ConfigSpec and in the new `serde` Deserializer, automatically apply
some "risky" conversions (e.g. long -> int) when it is not lossy, close
[TheElectronWill/night-config#119
- Allow custom `Config` storage in `ConfigSpec`, by
[@&#8203;PaintNinja](https://togithub.com/PaintNinja) in
[TheElectronWill/night-config#168
- More unit tests, on Java 8, Java 11 and Java 17, by
[@&#8203;TheElectronWill](https://togithub.com/TheElectronWill)

##### Bug fixes

- toml: Fix squeezed Files by
[@&#8203;Stein-N](https://togithub.com/Stein-N) in
[TheElectronWill/night-config#154
- toml: Align List ending when in a "group" by
[@&#8203;Stein-N](https://togithub.com/Stein-N) in
[TheElectronWill/night-config#160
- toml: Allow header comments by
[@&#8203;Stein-N](https://togithub.com/Stein-N) in
[TheElectronWill/night-config#162
- json: Disallow trailing data (configurable) by
[@&#8203;TheElectronWill](https://togithub.com/TheElectronWill), close
[TheElectronWill/night-config#167
- Multiple bugs have been fixed in `ObjectConverter` by
[@&#8203;TheElectronWill](https://togithub.com/TheElectronWill) in
[TheElectronWill/night-config#163

##### Compatibility Note

NightConfig 3.7.0 is backward-compatible with previous releases,
therefore I encourage you to upgrade your dependency on NightConfig in
order to benefit from the new version.

Binary-wise, the backward-compatibility has been checked with `japicmp`.
Source-wise, there is one small incompatible change:
`FileWatcher#addWatch` no longers throws an `IOException`, which can
result in a compiler error `unreachable code` on the associated
try/catch ([example in
forge](https://togithub.com/MinecraftForge/MinecraftForge/blob/640c206117864085e3c25889bcabeaddd555e9fb/fmlcore/src/main/java/net/minecraftforge/fml/config/ConfigFileTypeHandler.java#L50-L55)).

##### New Contributors

- [@&#8203;vaperion](https://togithub.com/vaperion) made their first
contribution in
[TheElectronWill/night-config#151
- [@&#8203;Stein-N](https://togithub.com/Stein-N) made their first
contribution in
[TheElectronWill/night-config#154
- [@&#8203;PaintNinja](https://togithub.com/PaintNinja) made their first
contribution in
[TheElectronWill/night-config#168

Thank you everyone!

**Full Changelog**:
TheElectronWill/night-config@v3.6.7...v3.7.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 6:00am" in timezone
Europe/Berlin, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/CloudNetService/CloudNet).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNjMuNSIsInVwZGF0ZWRJblZlciI6IjM3LjM2My41IiwidGFyZ2V0QnJhbmNoIjoibmlnaHRseSIsImxhYmVscyI6WyJ0OiBkZXBlbmRlbmNpZXMiXX0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

None yet

3 participants