Skip to content

Conversation

@rPraml
Copy link
Contributor

@rPraml rPraml commented Sep 23, 2024

In this PR I will try to optimize map access on several places and pick up @gbrail's comment: #1575 (comment)

@rPraml rPraml force-pushed the optimize-map-access branch from 5616739 to 972f5e2 Compare September 23, 2024 12:08
// Update the existing value and keep it in the same place in the list
map.get(nv).value = value;
}
map.compute(
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 will save one hash computation, but have the price of an additional invocation of a lambda expression:
I did this short test:

	@Test
	public void speed() {
		for (long i = 0L; i < 10_000_000L; i++) {
			ht.put("one", 1);
			ht.put("two", 2);
			ht.put("three", 3);
			ht.put("four", 4);
			ht.put("five", 5);
			ht.put("six", 6);
			ht.put("seven", 7);
			ht.put("eight", 8);
			ht.put("nine", 9);
			ht.put("ten", 10);

		}
	}

it runs with the old code in apporx 8,5sec and with the new code approx 5,5 sec

NativeJavaMethod setters = null;
String setterName = "set".concat(nameComponent);

if (ht.containsKey(setterName)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can safely remove containsKey here, as we call get in the next line, which is checked for null by the instanceOf test.

String getterName = prefix.concat(propertyName);
if (ht.containsKey(getterName)) {
// Check that the getter is a method.
Object member = ht.get(getterName);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same here. Do not hit map twice

@gbrail
Copy link
Collaborator

gbrail commented Sep 23, 2024

Yes, this is a good change. Thanks!

@gbrail gbrail merged commit 2eccdd1 into mozilla:master Sep 23, 2024
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