Skip to content

Improve getResources performance #9406

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

Jump to bottom
Merged
merged 1 commit into from
May 19, 2020
Merged

Improve getResources performance #9406

merged 1 commit into from
May 19, 2020

Conversation

@stuartwdouglas
Copy link
Collaborator

@stuartwdouglas stuartwdouglas commented May 18, 2020

No description provided.

@boring-cyborg boring-cyborg bot added the area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins label May 18, 2020
@stuartwdouglas
Copy link
Collaborator Author

stuartwdouglas commented May 18, 2020

Previous failures should be fixed by the changes in JarClassPathElement

@stuartwdouglas stuartwdouglas requested a review from geoand May 18, 2020 11:13
geoand
geoand approved these changes May 18, 2020
View reviewed changes
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

👍

@geoand
Copy link
Contributor

geoand commented May 18, 2020

Assuming CI passes, @gsmet we definitely want this one in for 1.5

@geoand
Copy link
Contributor

geoand commented May 18, 2020

Hm...

Weird errors:

2020-05-18T11:55:13.6450696Z [ERROR] COMPILATION ERROR : 
2020-05-18T11:55:13.6456050Z [INFO] -------------------------------------------------------------
2020-05-18T11:55:13.6464777Z [ERROR] error reading /home/runner/.m2/repository/net/bytebuddy/byte-buddy/1.10.5/byte-buddy-1.10.5.jar; zip END header not found
2020-05-18T11:55:13.6479210Z [ERROR] /home/runner/work/quarkus/quarkus/independent-projects/tools/common/src/test/java/io/quarkus/cli/commands/CreateProjectTest.java:[1,1] cannot access io.quarkus.cli.commands
2020-05-18T11:55:13.6481735Z   zip END header not found
2020-05-18T11:55:13.6493401Z [ERROR] /home/runner/work/quarkus/quarkus/independent-projects/tools/common/src/test/java/io/quarkus/maven/utilities/PomTransformerTest.java:[1,1] cannot access io.quarkus.maven.utilities
2020-05-18T11:55:13.6495858Z   zip END header not found
2020-05-18T11:55:13.6509928Z [ERROR] /home/runner/work/quarkus/quarkus/independent-projects/tools/common/src/test/java/io/quarkus/test/platform/descriptor/CombinedQuarkusPlatformDescriptorTest.java:[1,1] cannot access io.quarkus.test.platform.descriptor

@gsmet
Copy link
Member

gsmet commented May 18, 2020

My opinion on this one is that it either goes into CR1 or it gets moved to 1.6.

So let's try to merge it in CR1.

@gsmet gsmet added this to the 1.5.0 milestone May 18, 2020
Postremus
Postremus reviewed May 18, 2020
View reviewed changes
...jects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java Outdated
ClassPathElement[] providers = state.loadableResources.get(name);
if (providers != null) {
for (ClassPathElement element : providers) {
resources.add(element.getResource(nm).getUrl());
Copy link
Member

@Postremus Postremus May 18, 2020

Choose a reason for hiding this comment

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

I tried out my application with this pr. I get an NPE at this line now.
Drools 6.5.0.Final loads some resources.

Value of nm: /META-INF/drools.default.packagebuilder.conf
Value of name: META-INF/drools.default.packagebuilder.conf

element.getResource(nm) returns null.
element.getResource(name) returns a value.

Copy link
Member

@gsmet gsmet May 18, 2020

Choose a reason for hiding this comment

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

@stuartwdouglas I let you check that comment.

Copy link
Collaborator Author

@stuartwdouglas stuartwdouglas May 18, 2020

Choose a reason for hiding this comment

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

There was a typo, some places were using the original name that did not have the / removed

@stuartwdouglas stuartwdouglas merged commit bb228f1 into quarkusio:master May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gsmet gsmet gsmet left review comments

@geoand geoand geoand approved these changes

+1 more reviewer

@Postremus Postremus Postremus left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins

Projects

None yet

Milestone

1.5.0.CR1

Development

Successfully merging this pull request may close these issues.

4 participants

@stuartwdouglas @geoand @gsmet @Postremus