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

Use Java-NIO to write EclipsePreferences #8

Merged
merged 1 commit into from
Apr 12, 2022

Conversation

HannesWell
Copy link
Member

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

@iloveeclipse
Copy link
Member

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

@mickaelistria
Copy link
Contributor

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

@HannesWell
Copy link
Member Author

@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
Copy link
Member Author

HannesWell commented Apr 9, 2022

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
Copy link
Contributor

Can you reproduce this failure with a local build?

@vogella
Copy link
Contributor

vogella commented Apr 11, 2022

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

@vogella
Copy link
Contributor

vogella commented Apr 11, 2022

#9

vogella added a commit to vogellacompany/equinox.bundles that referenced this pull request Apr 11, 2022
vogella added a commit that referenced this pull request Apr 11, 2022
@HannesWell HannesWell merged commit bf16040 into eclipse-equinox:master Apr 12, 2022
@HannesWell HannesWell deleted the preferencesWrite branch April 12, 2022 07:51
@vogella
Copy link
Contributor

vogella commented Apr 12, 2022

Nice. Will it also be faster?

@HannesWell
Copy link
Member Author

Nice. Will it also be faster?

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

@vogella
Copy link
Contributor

vogella commented Apr 12, 2022

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

@merks
Copy link
Contributor

merks commented Apr 12, 2022

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

@vogella
Copy link
Contributor

vogella commented Apr 12, 2022

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

@ghost
Copy link

ghost commented Apr 12, 2022

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

no.
However i like to Files.writeString

@HannesWell
Copy link
Member Author

@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.

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

5 participants