-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Adding an example of an array type wrapper without loose of performance. #33420
Closed
Closed
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
eee4e9a
Adding an example of an array type wrapper without loose of perfomrance.
cf91a70
Merge branch 'master' of github.com:stakaz/julia
42e9afe
corrected the example
86434bb
removed trailing whitespaces
6e6b1c0
correctd the example text
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this should encourage the passing on of keywords, for named
A[i=3]
type indexing?Link: JuliaCollections/AxisArraysFuture#1 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keyword arguments often cause various performance problems. Just look at this for example: JuliaArrays/StaticArrays.jl#540 . I don't this it should be encouraged in a section dedicated to fast wrappers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. But is this a concern with unused
kw...
being passed along, or only when they are actually used? I can’t detect any effect on things I tried.(And, is there a good explanation of this issue somewhere?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem occurs when you call a function with keyword arguments. It's OK to have default keyword arguments though but even passing these default kwargs slows things down. That's why in some places keyword arguments are forwarded as normal arguments (as named tuples). There might be some exceptions that can be optimized by the compiler but from my experience it's a good rule of thumb.
I don't know, it seems to generally be hard to find good explanations of such compiler details.