Optimizing java code#65
Optimizing java code#65bloderxd wants to merge 2 commits intoclojure:masterfrom bloderxd:optimize-code
Conversation
|
The Clojure project does not accept pull requests. If you want to suggest proposed improvements, you can file a JIRA ticket here: http://dev.clojure.org/jira/browse/CLJ Also, I briefly scanned through your proposed changes, and it looks like reformatting some code with only changes in line breaks and/or white space? If so, I'll simply mention that such changes are typically low priority compared to other changes that fix bugs, improve run time performance, or add new features. |
|
Yes, it's not high priority but I saw java code and I thought very ugly, then I just would like to make it better to other people who will see it. |
|
and optimize this code doesn't involve just break lines and white spaces but also some if, switch logics |
|
@bloder You can open whatever tickets on JIRA for whatever changes you are interested in. I'm simply trying to tell you that prettyifying ugly code has been desired by many people besides you, and so far such changes have been considered very low priority. I don't make the decisions of what changes are made to Clojure and which are not -- just passing on some info about the past. |
|
@bloder we're not interested in these changes, sorry - please close the PR. |
|
Where are the optimizations? |
|
This is just an example to suggest the code optimization. |
|
This has an interesting side affect of making @bloder look like he's contributed to Clojure. Very sketchy. |
|
I just did these simple modifications because I saw they don't accept pull requests but I'd like to optimize other classes too, look this one - https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/EdnReader.java This PR is just to example what I'd like to do about clojure java code, it's better do this example and wait them answers than optimize all java code and they close my PR. |
|
You keep saying "optimize", but I don't think it means what you think it means. |
|
@bloder it seems they are too busy to fix those small code styling issues, as there are none "optimizations" being done, they would rather focus on other issues. Also, they don't accept contributions from here. I advise you look somewhere else to start contributing. @neverfox @zachncst I really see no need for this kind of comments, he's just trying to help. I actually think that his contributions makes sense and probably took some digging, perhaps he was reading the source and found it could be better. I maintain software and I don't mind merging typos in the readme. I think every contribution is equal, given that it makes sense for the project. Please be kind. |
|
@bloder fixing whitespace issues is not optimization. Do you have an example of where you would optimize the Java code? I believe if you had some real optimizations (again, whitespace changes are not optimizations) then you could get a bit more traction with your PR (albeit on Jira, not GitHub). |
|
@AeroNotix Ok, I'll try to refactor some big if or for or switch and make other commit here, ok? Sorry if I'm offending some one. |
|
No, clearly the maintainers don't want you to do that. Again, the difference between whitespace changes and optimizations is huge. Do not confuse the two. If you have some actual optimizations then head over to JIRA and explain which pieces of code need optimizing, optimize them, measure the optimization and compare to the unoptimized code. Whitespace changes are not optimizations. |
|
Yes I know that, my mistake in this PR was just remove whitespaces to example my idea, sorry about it, a good example that I saw was two or more for's doing the same thing in different functions changing only what ISec field will receive, my idea is optimize it creating a new solution to don't repeat it (DRY), like a function, I know my example was ugly but I hope you understand my idea, it's not about whitespaces, it's about code design. And don't worry I understood you don't accept contribution by github. |
|
@bloder I apologize for misinterpreting your intentions. I've come across a kind of PR troll before that is primarily trying to turn the PR into some sort of commentary on or outright ridicule of the project. In my haste, I took this to be something along those lines and I was wrong about that. In any case, good luck with your proposals over on JIRA. Cheers. @thiagofm |
|
@neverfox no problem. I also struggle with them. I guess it's always a good idea to think of others having good intentions upfront, I know it's hard, but I guess by doing that we can have a better community. |
|
@neverfox no problem, it's also my fault that could not express myself properly. |
Advanced debugger feature tests: - Conditional breakpoint fires on loop iterations (clojure#53) - One-shot breakpoint fires exactly once (clojure#54) - Breakpoint ignoreCount skips initial hits (clojure#55) - Breakpoint hit count tracks total activations (clojure#56) - DebugStackFrame.getScope() returns local variable scope (clojure#57) - DebugStackFrame.getLanguage() returns cloffle language info (clojure#58) - Internal frames not visible in default mode (clojure#59) - SuspendedEvent.getSuspendAnchor() returns BEFORE (clojure#60) - Return value available after step-over (clojure#61) - Breakpoint isResolved after execution (clojure#62) - Breakpoint enable/disable toggle (clojure#63) - SuspendedEvent.getBreakpoints() reports the firing breakpoint (clojure#64) - Step-into count > 1 steps multiple times (clojure#65) - Step-over count > 1 skips multiple statements (clojure#66) - isBreakpointHit() vs isStep() distinction (clojure#67) - Breakpoint on function in different source fires on call (clojure#68) - Breakpoint on defn line fires during definition (clojure#69) - Eval expression in suspended frame context (clojure#70) All 775 JUnit tests pass. Co-authored-by: karl <karl@medplum.com>
…agation (#6) * Improve debugger support: RootTag on FnDispatchNode, source section propagation, language reference on FnNode Key changes: - FnDispatchNode now reports RootTag alongside RootBodyTag, enabling the Truffle debugger to recognize function entry boundaries for step-into - FnDispatchNode gets its source section from the parent FnNode so that isInstrumentable() returns true - FnNode stores a language reference (set by ExprToNode) and uses it as primary source in getCallTarget(), falling back to Clojure.getContext() lookup. This ensures per-function ClojureRootNode instances are created with a proper language association for stack frame reporting. - InvokeNode propagates source sections to FnDispatchNode when creating static call targets - truffleEval() now sets source sections and root names on eagerly executed form roots - SequentialFormNode per-form roots now set source sections properly (full source first, then narrow when form section is available) - 8 new debugger tests (20 total, all passing) covering function name reporting, multi-line fn breakpoints, step-over, recursive stack growth, step-out, source section propagation, and eager eval roots - All 725 JUnit tests pass Co-authored-by: karl <karl@medplum.com> * Update CLOFFLE_NOTES.md with debugger improvements documentation Document the FnDispatchNode RootTag fix, source section propagation, language reference on FnNode, truffleEval source sections, and SequentialFormNode narrowing. Update known limitations to reflect current state. Co-authored-by: karl <karl@medplum.com> * Add StatementTag to hasTag on remaining call node classes InstanceCallNode, ProtocolInvokeNode, NewNode, NativeCallNode, and KeywordInvokeNode now report StandardTags.StatementTag alongside CallTag and ExpressionTag for instrumentation consistency. Co-authored-by: karl <karl@medplum.com> * Remove unnecessary @TruffleBoundary, add StatementTag to call nodes, restructure SequentialFormNode Major debugger improvements: - SequentialFormNode: Removed @TruffleBoundary from executeSequentially() and restructured to use @children DirectCallNode[] created at parse time. Per-form roots are now created in the constructor with proper source section narrowing (full source first, adoptChildren(), then narrow). This allows the debugger to step between top-level forms and enables breakpoints to match nodes inside per-form roots. - InvokeNode, GenericStaticCallNode: Added StatementTag alongside CallTag and ExpressionTag. Breakpoints default to matching StatementTag, so call expressions were previously invisible to line breakpoints. - ClojureNode: Removed @TruffleBoundary from getSourceSection() - this is metadata computation that doesn't need a compilation boundary. - ClojureTypes: Removed @TruffleBoundary from castDouble() - trivial cast. - Updated DebuggerTest: stepIntoFromCallSite now asserts 2 suspensions (step-into actually enters the called function body). Updated test comments. - All 725 JUnit tests pass. Co-authored-by: karl <karl@medplum.com> * Update CLOFFLE_NOTES.md: document StatementTag on call nodes, SequentialFormNode restructure, boundary removals Co-authored-by: karl <karl@medplum.com> * Add 20 new debugger tests (32 total) New tests cover: - Step-into named function reports callee source (clojure#13) - Step-into multi-arity function (clojure#14) - Breakpoint on call expression line with StatementTag (clojure#15) - Three breakpoints fire in order on separate def forms (clojure#16) - Step-into then step-over stays in callee body (clojure#17) - Step-into then step-out returns to caller (clojure#18) - Step-into anonymous fn created with fn form (clojure#19) - Breakpoint inside multi-line function body (clojure#20) - Step-into higher-order function call (clojure#21) - Recursive stack depth monotonically increases (clojure#22) - Step-out from function returns to caller (clojure#23) - Breakpoint on if branch fires (clojure#24) - Breakpoint on let binding fires (clojure#25) - Step-into closure that captures locals (clojure#26) - Breakpoint in loop body fires on each iteration (clojure#27) - Source section at call site has correct characters (clojure#28) - Breakpoint inside do body (clojure#29) - Step-into across separate eval contexts (clojure#30) - Breakpoint with cond macro expansion (clojure#31) - Multiple step-into follows b->c call chain (clojure#32) All 737 JUnit tests pass. Co-authored-by: karl <karl@medplum.com> * Add 20 more debugger tests (52 total) New tests cover: - Step-over does not enter callee (clojure#33) - Breakpoint inside try body (clojure#34) - Step-into variadic function (clojure#35) - Breakpoint on case form (clojure#36) - Breakpoint on throw form (clojure#37) - Breakpoint on recur form fires each iteration (clojure#38) - Source file name at breakpoint is correct (clojure#39) - Step-into locally-defined function (let + fn) (clojure#40) - Breakpoint on nested let (clojure#41) - Breakpoint removal prevents further hits (clojure#42) - Breakpoint on keyword invoke (:key map) (clojure#43) - Breakpoint inside letfn with mutual recursion (clojure#44) - Breakpoint on Java interop .method call (clojure#45) - Breakpoint on constructor call (clojure#46) - Breakpoint on static method call (clojure#47) - Step-into multi-arity with delegating arity (clojure#48) - Continue after multiple breakpoints resumes fully (clojure#49) - Breakpoint on and/or macro expansion (clojure#50) - Source section length matches form length (clojure#51) - Breakpoint on when macro (clojure#52) All 757 JUnit tests pass. Co-authored-by: karl <karl@medplum.com> * Add 18 advanced debugger tests (70 total) Advanced debugger feature tests: - Conditional breakpoint fires on loop iterations (clojure#53) - One-shot breakpoint fires exactly once (clojure#54) - Breakpoint ignoreCount skips initial hits (clojure#55) - Breakpoint hit count tracks total activations (clojure#56) - DebugStackFrame.getScope() returns local variable scope (clojure#57) - DebugStackFrame.getLanguage() returns cloffle language info (clojure#58) - Internal frames not visible in default mode (clojure#59) - SuspendedEvent.getSuspendAnchor() returns BEFORE (clojure#60) - Return value available after step-over (clojure#61) - Breakpoint isResolved after execution (clojure#62) - Breakpoint enable/disable toggle (clojure#63) - SuspendedEvent.getBreakpoints() reports the firing breakpoint (clojure#64) - Step-into count > 1 steps multiple times (clojure#65) - Step-over count > 1 skips multiple statements (clojure#66) - isBreakpointHit() vs isStep() distinction (clojure#67) - Breakpoint on function in different source fires on call (clojure#68) - Breakpoint on defn line fires during definition (clojure#69) - Eval expression in suspended frame context (clojure#70) All 775 JUnit tests pass. Co-authored-by: karl <karl@medplum.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
7 participants
What's this?
First of all... Good job about clojure!
I'd like to make clojure code better, then I did some optimization in Fn interface and abstract class, if you approve it, I can also optimize other classes.