search

eclipse-equinox / equinox.bundles

Eclipse Public License 2.0
8 stars 16 forks source link

Use Java-NIO to write EclipsePreferences #8

Closed HannesWell closed 2 years ago

HannesWell commented 2 years ago

This PR aims to simplify the write of Eclipse-Preferences by using Java-NIO, which makes SafeFileOutputStream obsolete.

iloveeclipse commented 2 years ago

@HannesWell : where's "Check Code Freeze Period" check? Shouldn't that now be executed?

mickaelistria commented 2 years ago

This commit may need to be rebased for workflow updates to be available.

HannesWell commented 2 years ago

@HannesWell : where's "Check Code Freeze Period" check? Shouldn't that now be executed?

They should indeed be executed. For PDE this already works (see my latest PR there). I suspect this is because GH Actions are not yet activated for all Equinox projects at all. But I have already asked the Webmaster-team to enabled them: https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/1148

So until the next freeze-period they should be available.

HannesWell commented 2 years ago

But what makes me wonder is the compilation failure in the build:

[ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:3.0.0-SNAPSHOT:compile (default-compile) on project org.eclipse.equinox.preferences: Compilation failure: Compilation failure: 
[ERROR] /home/jenkins/agent/workspace/rt.equinox.bundles_PR-8/bundles/org.eclipse.equinox.preferences/src/org/eclipse/core/internal/preferences/EclipsePreferences.java:[290] 
[ERROR]     Files.writeString(tmp, fileContent, StandardCharsets.UTF_8);
[ERROR]           ^^^^^^^^^^^
[ERROR] The method writeString(Path, String, Charset) is undefined for the type Files
[ERROR] /home/jenkins/agent/workspace/rt.equinox.bundles_PR-8/bundles/org.eclipse.equinox.preferences/src/org/eclipse/core/internal/preferences/EclipsePreferences.java:[293] 
[ERROR]     Files.writeString(preferenceFile, fileContent, StandardCharsets.UTF_8);
[ERROR]           ^^^^^^^^^^^
[ERROR] The method writeString(Path, String, Charset) is undefined for the type Files
[ERROR] /home/jenkins/agent/workspace/rt.equinox.bundles_PR-8/bundles/org.eclipse.equinox.preferences/src/org/eclipse/core/internal/preferences/EclipsePreferences.java:[306] 
[ERROR]     String string = output.toString(StandardCharsets.UTF_8);
[ERROR]                            ^^^^^^^^
[ERROR] The method toString(String) in the type ByteArrayOutputStream is not applicable for the arguments (Charset)
[ERROR] 3 problems (3 errors)

The marked methods where all introduced in Java-10/11 so they should be available.

The BREE and the jdt-preferences of the project are all at Java-11

mickaelistria commented 2 years ago

Can you reproduce this failure with a local build?

vogella commented 2 years ago

Looks to me that build.properties still enforces Java 8, I push a separate PR for this.

vogella commented 2 years ago

https://github.com/eclipse-equinox/equinox.bundles/pull/9

vogella commented 2 years ago

Nice. Will it also be faster?

HannesWell commented 2 years ago

Nice. Will it also be faster?

Probably not significantly faster, because the general IO-operations are still performed. :/

vogella commented 2 years ago

@HannesWell Android offers a async way to persist perferences. Maybe we should add this also to Equinox?

merks commented 2 years ago

I'm a bit doubtful that saving preferences needs optimization...

vogella commented 2 years ago

@jukzi did you every see deplay due to preference saves?

ghost commented 2 years ago

@jukzi did you every see deplay due to preference saves?

no. However i like to Files.writeString

HannesWell commented 2 years ago

@HannesWell Android offers a async way to persist perferences. Maybe we should add this also to Equinox?

I agree with the others that it is likely not worth the effort. In general avoiding IO-operations is usually much more beneficial than just parallelizing it. Therefore I would spend my time and code-complexity there.