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

#2716: tuple optimization #2898

Merged

Conversation

levBagryansky
Copy link
Member

@levBagryansky levBagryansky commented Feb 27, 2024

Relates to #2716
This pr makes tuple.at faster. Testing time decreased by 2 times on my laptop.
Such test became more than 10 times faster:

#Test
[] > just-long-test
  seq > @
    *
      TRUE
      TRUE
      TRUE
      TRUE
      TRUE

PR-Codex overview

This PR adds new tests and a method at-without-checks to the tuple object in the eo-runtime.

Detailed summary

  • Added tests for at-without-checks method with first and last element
  • Added at-without-checks method to tuple object for retrieving elements without index validation

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@levBagryansky
Copy link
Member Author

@maxonfjvipon please check

eo-runtime/src/main/eo/org/eolang/tuple.eo Outdated Show resolved Hide resolved
eo-runtime/src/main/eo/org/eolang/tuple.eo Outdated Show resolved Hide resolved
eo-runtime/src/main/eo/org/eolang/tuple.eo Outdated Show resolved Hide resolved
^.at.at-without-checks index

# Takes element from the tuple without checking i.
# i must be less than length and greater or equal 0.
Copy link
Member

Choose a reason for hiding this comment

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

@levBagryansky i actually may be any object, it's not mandatory i to be int. It can be for example

[] > my-i
  [x] > lt
    TRUE > @

And it will work. So I would say that it's expected i has lt attribute and we don't check its value

Copy link
Member Author

Choose a reason for hiding this comment

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

@maxonfjvipon i should be int. Otherwise the object does not have sense

Copy link
Member

Choose a reason for hiding this comment

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

@levBagryansky It should not and we can't guarantee it. All we need that i has attribute lt with 1 free attribute. That's it

Copy link
Member Author

Choose a reason for hiding this comment

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

What will be (* 1 2 3) .at my-int here? Not ub but something like that, because then the method guarantees nothing

Copy link
Member Author

Choose a reason for hiding this comment

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

@maxonfjvipon Also I did not write that i should be int. i must be less than length and greater or equal 0. That is all. If you object is less than length and greater or equal to 0 then you can use this method.

Copy link
Member

Choose a reason for hiding this comment

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

@levBagryansky but x.lt y is not supposed to mean "x is less than y"

Copy link
Member Author

Choose a reason for hiding this comment

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

Supposed to be. Actually it is abstraction. We have polymorphism here.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can remove it if it seems confusing

Copy link
Member

@maxonfjvipon maxonfjvipon Feb 27, 2024

Choose a reason for hiding this comment

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

@levBagryansky I would propose something like this for the description:

"Takes element from the tuple without index validation.
Here i must be an object that can be comparable with int using lt method."

Copy link
Member Author

Choose a reason for hiding this comment

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

@levBagryansky
Copy link
Member Author

@yegor256 please check

1 similar comment
@levBagryansky
Copy link
Member Author

@yegor256 please check

@yegor256
Copy link
Member

@rultor merge

@yegor256
Copy link
Member

@rultor status

@rultor
Copy link
Contributor

rultor commented Feb 29, 2024

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

@rultor
Copy link
Contributor

rultor commented Feb 29, 2024

@rultor status

@yegor256 This is what's going on here:

  • request 1970587478 is in processing, command is merge
  • request has 6 parameter(s):
  • fork: [email protected]:levBagryansky/eo.git
  • fork_branch: 2716_tuple-optimization-in-eo
  • head: [email protected]:objectionary/eo.git
  • head_branch: master
  • pull_id: 2898
  • pull_title: #2716: tuple optimization
  • build is not running

More information about Rultor commands you can get here.

@rultor rultor merged commit 5c815fa into objectionary:master Feb 29, 2024
15 checks passed
@levBagryansky levBagryansky deleted the 2716_tuple-optimization-in-eo branch February 29, 2024 13:13
@rultor
Copy link
Contributor

rultor commented Feb 29, 2024

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 24min)

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.

4 participants